Repository navigation
compute: borrow each correction chunk once per merge, not once per update - #38896
Conversation
f688154 to
72afe41
Compare
72afe41 to
cde2ae6
Compare
cde2ae6 to
0266dbb
Compare
|
Addressed. The 🤖 Posted by Claude Code |
|
The description says this fixes the k-way merge, but Either hoist there too (keep a view per |
0266dbb to
ca33cdf
Compare
|
Good catch, thanks. I hoisted the borrow in the k-way merge rather than narrowing the description. The heap orders updates directly, so 🤖 Posted by Claude Code |
…date Every element access in `CorrectionV2` re-decoded its chunk. `Chunk::index` was `self.column().borrow().get(i)`, and `Column::borrow` on a serialized column rebuilds the struct-of-arrays view from the serialized header, so a k-way merge paid O(log k) header decodes per output update: 42.7% of an insert profile, ahead of the merge itself at 12.2%. Borrowing is now hoisted to one view per chunk, or per chunk pair in the two-way merge, whose inner loop runs until either cursor crosses a chunk boundary. The k-way merge that reads over three or more chains works in rounds the same way: each round borrows the current chunk of every input and heaps their updates, and ends when an input crosses a chunk boundary. The heap orders updates directly, so `MergeHeap` and `MergeCursor` are gone. `Cursor` gained `chunk_handle`, `reads_from`, and `get_with` for callers that hold a view across cursor steps. `get` and `index` remain the documented single-access forms. Chain iteration, chunk splitting, and the time search index a hoisted view likewise. Holding a view inside the iterator that `updates_before` hands across the persist writer's `await` needs the view to be `Send`, which is a new bound on the container. The container bounds move into a `DataContainer` helper trait so `Data` stays readable. Insert improves 1.6x on a 4.19M-update append workload, and 2.0x under a memory cap, where each redundant decode also faulted the header page back in. The new `column_borrow` bench in `mz-timely-util` measures that decode: per-element borrows of a serialized column against one hoisted borrow and a typed column. `merge_many_consolidates_across_chunks` checks a three-way merge over chains of several chunks each. Co-Authored-By: Claude Fable 5.1 <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
ca33cdf to
b736ed7
Compare
`compute_correction_v2_chunk_size` sizes `CorrectionV2`'s staging area, not its chunks: chunk bodies are fixed at the columnar ship size. The staging size sets how many updates accumulate before a chain is minted, and every chain minted is a chain some later read has to merge. Raising the default from 8 KiB to 2 MiB, one chunk body's worth, takes insert on a 4.19M-update append workload from 3503 ms to 1897 ms, with drain and peak RSS unchanged. These are medians of three `correction_mem` runs with the rest of the stack applied and no pool installed. The curve is monotone across the range (the new `bench_chunk_size` sweep in the `correction` bench shows it), so the default is bounded only by the per-sink, per-worker cost of the staging vector. The stage counted its fill in tuples of `size_of::<(D, Timestamp, Diff)>()`, which is 40 bytes for a `Row` however wide the row is, so a byte-sized flag bounded neither the staging memory nor the reported size once rows spill out of line. A new `DataBytes` trait on the buffer's data types reports each datum's bytes with the heap included (`Row` via `byte_len`, `DataflowErrorSer` via its serialized length), the stage ships when the staged bytes reach the flag, and its size metrics carry the same figure. `stage_ships_by_bytes` checks that rows of two widths ship after the number of updates their bytes predict. That vector was allocated eagerly at the configured capacity and re-allocated at that capacity on every flush. At 2 MiB per sink and worker that is no longer free, so it now grows on demand and gives its allocation back on flush. The mzcompose and parallel-workload flag tables follow the new default, since CI runs the value listed there rather than the compiled default. The flag's description now says what it sizes. First of four, split out of #38645: this, the O(1) metrics (#38895), the per-chunk borrow hoist (#38896), and the pool substrate (#38897) land in that order. Supersedes the fork PR #38647. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <[email protected]>
Every element access in
CorrectionV2re-decoded its chunk.Chunk::indexwasself.column().borrow().get(i), andColumn::borrowon a serialized column rebuilds the struct-of-arrays view from the serialized header, so a k-way merge paid O(log k) header decodes per output update: 42.7% of an insert profile, ahead of the merge itself at 12.2%.Borrowing is now hoisted to one view per chunk, or per chunk pair in the two-way merge, whose inner loop runs until either cursor crosses a chunk boundary. The k-way merge that reads over three or more chains works in rounds the same way: each round borrows the current chunk of every input and heaps their updates, and ends when an input crosses a chunk boundary.
merge_many_consolidates_across_chunkscovers it.Cursorgainedchunk_handle,reads_from, andget_withfor callers that hold a view across cursor steps;getandindexremain the documented single-access forms. Chain iteration, chunk splitting, and the time search index a hoisted view likewise.Holding a view inside the iterator that
updates_beforehands across the persist writer'sawaitneeds the view to beSend, which is a new bound on the container. The container bounds move into aDataContainerhelper trait soDatastays readable.Insert improves 1.6x on a 4.19M-update append workload, and 2.0x under a memory cap, where each redundant decode also faulted the header page back in. The new
column_borrowbench inmz-timely-utilmeasures that decode: per-element borrows of a serialized column against one hoisted borrow and a typed column.Supersedes the fork PR #38649.
🤖 Generated with Claude Code