ROADMAP: write up the remaining performance work
Five items in dependency order, each sized to land on its own. The design decisions already settled are recorded so they are not re-derived, and so are the ordering constraints, which are the part that actually matters -- notably that the _id index must follow the tree, because it updates on every insert and against a sorted array that is only affordable while ids happen to append at the end. Also carries the ground rules the earlier work established: A/B on one harness rather than trusting a model, mutation-check tests that guard an invariant, and the two absolutes (an index may only over-approximate; the database must always open). Each item names the traps found while investigating it -- deletion being where B+trees go wrong, listIndexes possibly noticing a real _id_ index, hashing compressed rather than uncompressed bytes, the fabricated Document values that break when Document changes meaning, and the durability guarantee that quietly weakens under group commit.
This commit is contained in:
@@ -216,6 +216,9 @@ above is recorded in `tests/e2e/results/phase1.txt`.
|
|||||||
|
|
||||||
### What is left (highest impact first)
|
### What is left (highest impact first)
|
||||||
|
|
||||||
|
Each is written up with its design decisions, ordering constraints and
|
||||||
|
traps in [ROADMAP.md](ROADMAP.md).
|
||||||
|
|
||||||
1. **Compress the log** — the largest remaining gap (×11). Payloads are
|
1. **Compress the log** — the largest remaining gap (×11). Payloads are
|
||||||
stored raw. A block-framed format with an LZ4 block codec would shrink
|
stored raw. A block-framed format with an LZ4 block codec would shrink
|
||||||
highly compressible workloads massively; note Zig 0.16 ships zstd
|
highly compressible workloads massively; note Zig 0.16 ships zstd
|
||||||
|
|||||||
197
ROADMAP.md
Normal file
197
ROADMAP.md
Normal file
@@ -0,0 +1,197 @@
|
|||||||
|
# Remaining performance work
|
||||||
|
|
||||||
|
Five items, in dependency order. Each is sized to be landed and verified on
|
||||||
|
its own; the ordering constraints between them are the load-bearing part, so
|
||||||
|
read those before picking one up.
|
||||||
|
|
||||||
|
Current numbers and what they mean are in the README; the recorded baseline
|
||||||
|
is `tests/e2e/results/phase1.txt`, reproduced with
|
||||||
|
`bash tests/e2e/compare-run.sh 1g 16k`.
|
||||||
|
|
||||||
|
## Ground rules
|
||||||
|
|
||||||
|
These held for everything already done and should hold here.
|
||||||
|
|
||||||
|
- **Measure A/B on one harness. Do not trust the model.** Several predictions
|
||||||
|
made while planning this work were wrong in both directions: CRC32 cost
|
||||||
|
twice what was estimated, one compaction fix turned out to be numerically
|
||||||
|
identical to the code it replaced, and a matcher change that measured 1.3x
|
||||||
|
in isolation did not move its benchmark row at all. The cheap way to A/B is
|
||||||
|
to flip one line, rebuild, run, flip it back.
|
||||||
|
- **Mutation-check any test guarding an invariant.** Break the thing the test
|
||||||
|
is supposed to catch and confirm it goes red. Several tests here were
|
||||||
|
written, looked reasonable, and only proved to have teeth after this.
|
||||||
|
- **The index invariant is absolute** (`src/index.zig` header): an index only
|
||||||
|
generates candidates, and the full filter is re-applied afterwards.
|
||||||
|
Over-approximating is slow. Under-approximating is a wrong answer.
|
||||||
|
- **The database must always open.** A unique index that finds duplicates in
|
||||||
|
existing data warns and keeps going; replay never refuses to start over
|
||||||
|
recoverable damage.
|
||||||
|
|
||||||
|
Verification for every item:
|
||||||
|
|
||||||
|
```sh
|
||||||
|
zig build test # unit, ReleaseFast
|
||||||
|
zig build test -Doptimize=ReleaseSafe # again with safety checks on
|
||||||
|
zig build # NOTE: `zig build test` does not refresh the binary
|
||||||
|
node tests/e2e/e2e6.js # 72 checks, self-contained, includes kill -9
|
||||||
|
```
|
||||||
|
|
||||||
|
Anything touching the write path or the log format also needs the crash pair
|
||||||
|
(`e2e2.js crash-a`, `kill -9`, restart, `e2e2.js crash-b`), and anything
|
||||||
|
touching indexes needs `e2e3.js` and `e2e4.js`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 1. B-tree over the encoded keys
|
||||||
|
|
||||||
|
**Why.** Index entries live in one sorted array, so inserting an entry
|
||||||
|
memmoves the tail. Building an index is fine (entries are appended and sorted
|
||||||
|
once) and removal is fine (regenerated and found by binary search), but
|
||||||
|
*inserting into a collection that already has an index* is quadratic. It also
|
||||||
|
blocks item 2.
|
||||||
|
|
||||||
|
**What.** Replace `Index.entries` with a B+tree whose nodes live in a flat
|
||||||
|
`ArrayListUnmanaged(Node)` addressed by `u32`. Slotted 4 KiB pages; keys
|
||||||
|
longer than a quarter of a node spill to an overflow slab (BSON strings reach
|
||||||
|
16 MB, so this is not optional). Leaves linked for ordered iteration.
|
||||||
|
|
||||||
|
The flat `u32`-indexed node array is a deliberate choice over pointer-linked
|
||||||
|
nodes: it makes the whole index one contiguous byte range that item 4 can
|
||||||
|
write into a checkpoint and read back without rebuilding.
|
||||||
|
|
||||||
|
**Keep.** `append_doc_entries` + `finish_bulk` become bulk leaf packing
|
||||||
|
(sort, fill leaves, build interior levels bottom-up). `remove_doc`'s
|
||||||
|
regenerate-and-search approach carries over unchanged — only the lookup
|
||||||
|
underneath it changes.
|
||||||
|
|
||||||
|
**Watch.** Deletion is where B+trees get subtly wrong. Not rebalancing on
|
||||||
|
delete (leaving underfull leaves, letting compaction reclaim them) is a
|
||||||
|
legitimate simplification and much easier to get right; take it unless
|
||||||
|
there is a reason not to.
|
||||||
|
|
||||||
|
**Tests that must keep passing, unchanged:** the randomized `lookup_range`
|
||||||
|
check against a brute-force filter, and the `remove_doc`-versus-scan
|
||||||
|
differential. Both already exist and both are mutation-checked.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 2. Ordered `_id` index — **depends on 1**
|
||||||
|
|
||||||
|
**Why.** `sort({_id: ...})` still materializes every candidate, and integer
|
||||||
|
`_id`s still fall back to a full collection scan on every `findOne`,
|
||||||
|
`updateOne` and `deleteOne`.
|
||||||
|
|
||||||
|
**What.** Give `Collection` an index over the encoded `_id`, maintained in
|
||||||
|
`upsert`, `evict_doc` and `apply_record`. Every document has an `_id` and it
|
||||||
|
is not sparse, so entry count equals document count and a full index scan
|
||||||
|
cannot miss a document — which is what the sort planner's full-scan plan
|
||||||
|
requires.
|
||||||
|
|
||||||
|
Then delete `value_fast_path_safe` and friends (`src/index.zig`). They exist
|
||||||
|
only because `serialize_value` gives `int32 1`, `int64 1` and `double 1.0`
|
||||||
|
different bytes despite comparing equal. `bson.encode_key` already gives them
|
||||||
|
identical bytes, so the guard is obsolete.
|
||||||
|
|
||||||
|
**Why it depends on item 1.** This index updates on *every* insert. Against a
|
||||||
|
sorted array that is a tail memmove each time — roughly 51 GB of memmove over
|
||||||
|
65,536 documents. It only looks acceptable because ObjectIds increase
|
||||||
|
monotonically and therefore append at the end; random or descending `_id`s
|
||||||
|
would collapse bulk insert, currently the project's best result. Do not land
|
||||||
|
this against the array without an explicit, documented "append-ordered `_id`s
|
||||||
|
only" caveat.
|
||||||
|
|
||||||
|
**Watch.** A real `_id_` index may start appearing in `listIndexes` and
|
||||||
|
writing an `index_create` record to the log. `e2e3.js` asserts on index
|
||||||
|
listings — check it before assuming this is invisible.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 3. Block-framed compressed log
|
||||||
|
|
||||||
|
**Why.** The largest remaining gap: 1.0 GB on disk against MongoDB's 93 MB,
|
||||||
|
because payloads are stored raw. Breaking the format is fine.
|
||||||
|
|
||||||
|
**What.** A file header plus a sequence of blocks; each block holds the
|
||||||
|
existing record framing, so `Engine.apply_record` does not change. Target
|
||||||
|
~256 KiB per block, and records never straddle blocks.
|
||||||
|
|
||||||
|
**The hash covers the compressed bytes, as they sit on disk.** If it covered
|
||||||
|
the uncompressed bytes you would have to run the decompressor over
|
||||||
|
possibly-corrupt input before you could validate it, leaning on the
|
||||||
|
decompressor's error handling for durability. Hashing the stored bytes means
|
||||||
|
the decompressor only ever sees input already proven intact.
|
||||||
|
|
||||||
|
**Torn tail versus interior corruption must stay distinguishable**, exactly as
|
||||||
|
today: a short read, an impossible length, or a hash mismatch *in the final
|
||||||
|
block* means a crash mid-append — truncate and return cleanly. A hash
|
||||||
|
mismatch anywhere else is `error.InvalidLog`.
|
||||||
|
|
||||||
|
**Codec: write an LZ4 block compressor** (~200 lines). Zig 0.16 ships zstd
|
||||||
|
*decompression only*, so zstd needs vendoring; `std.compress.flate` has a
|
||||||
|
compressor but deflate level 1 runs slower than the current insert rate, so
|
||||||
|
it would make writes worse to make the disk smaller. Keep a codec byte in the
|
||||||
|
header so raw stays legal (needed anyway when compression does not help) and
|
||||||
|
zstd can be swapped in later.
|
||||||
|
|
||||||
|
**Watch.** `Engine.compact` goes through the same `Log` API and so gets
|
||||||
|
compression for free — but it must still defer syncing and commit once.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 4. Stop giving every document its own arena
|
||||||
|
|
||||||
|
**Why.** Two gaps at once. RSS is 2.0 GB against 1.4 GB for a 1.0 GB dataset
|
||||||
|
because each document carries an `ArenaAllocator` and a second full copy of
|
||||||
|
its data as a `Pair` tree. And the range-scan gap is *not* the matcher — it is
|
||||||
|
walking 65,536 documents that each live in a separate allocation, one pointer
|
||||||
|
chase apiece.
|
||||||
|
|
||||||
|
**What.** Store canonical BSON bytes in a per-collection slab; the docs map
|
||||||
|
holds a reference into it. Match against those bytes directly, walking the
|
||||||
|
element stream and skipping by length any field the filter does not name — for
|
||||||
|
the benchmark document that is ~40 bytes touched instead of 16 KB. Add a
|
||||||
|
borrowed parse (spine only, pointing into the stored bytes) for sort,
|
||||||
|
projection and update. `emit_docs` with no projection becomes a memcpy.
|
||||||
|
|
||||||
|
**Depends on item 1** in spirit: index entry keys no longer alias document
|
||||||
|
arenas (that was fixed when entries moved to encoded bytes), but `Entry.id`
|
||||||
|
still aliases the docs map key, and that aliasing needs to be understood
|
||||||
|
before documents start moving between slabs.
|
||||||
|
|
||||||
|
**Watch.** This is the deepest change of the five and touches every test.
|
||||||
|
Land the byte matcher behind a flag and differential-test it against the
|
||||||
|
existing matcher on a corpus before switching over. Also replace the
|
||||||
|
fabricated `bson.Document{ .arena = undefined, ... }` values scattered through
|
||||||
|
`query.zig` and `commands.zig` with an explicit view type — they work today
|
||||||
|
only because nothing calls `deinit` on them, and they are exactly what breaks
|
||||||
|
when `Document` changes meaning.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 5. Decompose the global lock
|
||||||
|
|
||||||
|
**Why.** One reader/writer lock covers the entire engine and is held across
|
||||||
|
fsync, compaction and reply construction, so writes cannot use more than one
|
||||||
|
core.
|
||||||
|
|
||||||
|
**What.** A catalog lock over the database and collection maps, plus a lock
|
||||||
|
per collection. Ordering is catalog then collection, never the reverse, and
|
||||||
|
never two collection locks at once — the only cross-collection operations are
|
||||||
|
the TTL sweep and compaction, and both must take collections one at a time.
|
||||||
|
Then cross-connection group commit: whichever writer finds no commit in
|
||||||
|
flight becomes the leader, seals the buffer, compresses, writes and syncs
|
||||||
|
once for everyone waiting.
|
||||||
|
|
||||||
|
**Before touching anything, enumerate what the current code relies on.**
|
||||||
|
`Entry.id` aliases the docs map key. `Engine.dup_index` is engine-global state
|
||||||
|
written by one call and read by the next. `Log.scratch` is a single shared
|
||||||
|
buffer justified by appends being single-writer. Collections are stored *by
|
||||||
|
value* in a hash map, so any insert can move them — that one needs fixing
|
||||||
|
first regardless.
|
||||||
|
|
||||||
|
**A durability guarantee changes here, and it should be a decision, not a
|
||||||
|
surprise.** Today the log always describes at least as much as memory —
|
||||||
|
strictly stronger than MongoDB. Under leader/follower commit a reader can
|
||||||
|
observe a write before its fsync completes, which is ordinary `w:1, j:true`
|
||||||
|
semantics. Check what `e2e2.js crash-b` actually asserts before changing it.
|
||||||
Reference in New Issue
Block a user