Open
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Introduces fibre/internal/row, a bucketed allocator of fixed-shape row batches used by the blob encode path and the rsema1d codec's work buffers. Replaces the per-encode sync.Pool with explicit retention (aged eviction, idle-grace drop) and mmap-backed regions above 1 MiB, keeping steady-state RSS proportional to concurrent in-flight encodes rather than worst-case per-worker reservation. Allocations run without holding the pool lock so a fresh mmap doesn't stall concurrent Gets/Puts behind a multi-ms syscall. row.Assembler layers a K+N row view on top of the pool: original rows alias input data zero-copy where possible, parity+head+tail come from a single pooled batch released as one unit. ProtocolParams.CodecWorkRows() exposes leopard-GF16's work-row count so callers size the pool without pool code needing to know codec internals. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wondertan
force-pushed
the
feat/slab-pool-allocator-alt
branch
from
6fbf0b1 to
e5cb881
Compare
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
1 participant
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.
Third and final iteration of the encoding memory layout for Fibre.
The 2nd iteration (#7091) rested on the intuition that freeing the rows for each validator as soon as
Uploadfinished with them would keep peak memory lower than holding the full blob's rows through the tail-latency drain. SinceUploadreturns after 2/3 of validators have acked, carrying all 128 MiB through the remaining tail felt obviously wasteful.In practice, this was a premature optimization that generated more complexity than it repaid. Per-validator releases are adversarial to the allocator: the rows that go free at any moment are a random subset of the blob, so the freed memory lands as fragmented holes rather than reusable slots. During implementation, it became clear that fragmentation is an issue with several rounds of optimization layered on top to compensate; however, they were only putting complex makeup on a pig.
In a sync review, @walldiss flagged the complexity of the slab allocator as an issue that reduces trust in it. We agreed to eliminate per-validator releases to reduce complexity. It was a great call that also confirmed the optimization was flawed in the end.
The 3rd iteration drops per-validator release entirely in favor of whole-batch pooling. It is significantly simpler, and more importantly, it behaves better under load: steady-state memory tracks the count of concurrent in-flight encodes rather than worker count × blob size. For example, 10 workers × 128 MiB blobs no longer pin ~10 GiB of work buffers; memory settles around whatever the network bottleneck is.
This steady state never emerged in the 2nd iteration because fragmented reuse forced fresh allocation for nearly every encode, and the allocator couldn't recycle a random scatter of freed rows into a contiguous batch-shaped request, so memory grew until every worker effectively held its own reservation.