Repository navigation
Conversation
This was referenced Sep 4, 2026
antiguru
marked this pull request as ready for review
September 14, 2026 18:02
antiguru
force-pushed
the
moritz/correction-borrow-hoist
branch
from
September 14, 2026 20:17
25d5cdc to
c4b20f7
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 3443 ms to 1860 ms at an unchanged peak RSS. The curve is monotone across the range, so the default is bounded only by the per-sink, per-worker cost of the staging vector, and the `correction` bench gains a `bench_chunk_size` sweep to show it. The flag's description now says what it sizes. 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, the stage ships when the staged bytes reach the flag, and its size metrics carry the same figure. `Row` answers with `byte_len`, `DataflowErrorSer` with its serialized length. The staging 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 grows on demand instead 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. Co-Authored-By: Claude Fable 5.1 <[email protected]>
`CorrectionV2::update_metrics` walked every chain and every chunk on every operation, from four call sites including insert, so its cost grew with the buffer: 17% of insert at 16.8M updates and rising. An `Accounting` handle, shared by the buffer and by every structure that holds its chains, now carries running record, size, and allocation totals, so a metrics update reads three counters. The totals hook the discipline the introspection logging already kept: a chain is announced once when the buffer takes it and retired once before it is dropped or consumed, so the totals and the logging cannot drift apart. Each chunk caches its serialized size at construction, and each chain maintains the sum as chunks are pushed, so no accounting step pages a chunk in. The reported size is the logical serialized size of the bodies, resident or not. The previous walk reported resident bytes only, which depended on what the pager had done with each chunk since. Residency belongs to the pager, which changes it without telling the buffer. Capacity is reported equal to size, since chunk bodies are exactly sized when minted, and allocations count one per chunk plus the staging vector while it holds one. `metrics_totals_match_walk` checks the totals against a walk after inserts, a partial drain, a since advance, and a consolidation. Co-Authored-By: Claude Fable 5.1 <[email protected]>
…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. `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. Co-Authored-By: Claude Fable 5.1 <[email protected]>
antiguru
force-pushed
the
moritz/correction-borrow-hoist
branch
from
September 14, 2026 20:26
c4b20f7 to
f688154
Compare
Member
Author
|
Closed in favor of #38896, the same branch as a native stacked PR in this repository (stack #38898). Fork PRs cannot join GitHub stacks. 🤖 Posted by Claude Code |
antiguru
added a commit
that referenced
this pull request
Sep 30, 2026
…date (#38896) 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. `merge_many_consolidates_across_chunks` covers it. `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. Supersedes the fork PR #38649. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #38648. Review the last commit; the diff shows #38647 and #38648 as well until they merge.
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.
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.
🤖 Generated with Claude Code