Conversation
Hasher.Write spawned a goroutine for every eigentree of four or more chunks. A Write's trees form a cascade — a 64 KiB write at counter 0 is [32,16,8,4,2,1] chunks, the last chunk held back — so a mid-sized write scattered ~10 µs jobs across threads and a 1 MiB one made ~150 allocations, most of them goroutine and scratch overhead. And guts.CompressEigentree fanned every large tree out one goroutine per 16 KiB group with no cap: 64 goroutines for a 1 MiB tree. The audit's suggested fix (compress trees up to MaxSIMD chunks inline, spawn only for larger ones) measures as a 1.7× regression on the 64 KiB case it cites: the cascade's small trees sum to nearly the size of its largest, and running them serially after the big one loses exactly the overlap the old code was — inefficiently — buying. So the decision is made on the write's total tree bytes instead. Below 32 KiB every tree runs inline and its CV is pushed at once: no goroutines, no scratch, nothing for a closure to capture and drag onto the heap (an earlier draft's "stack" CV array escaped precisely that way). Above it, each large tree gets a goroutine and each run of small trees shares one, so the tail overlaps the big trees; CVs are pushed in tree order once all are in, since the CV stack merges depend on that order. CompressEigentree keeps 16 KiB per goroutine — 32 KiB measured ~10% slower on 64–128 KiB trees — but deals groups in contiguous runs capped at NumCPU, and runs serially when a tree does not split two ways. Both thresholds are from a sweep on the generic path (darwin/arm64): 16 KiB at the Write level made a 32 KiB write slower parallel than serial; 64 KiB left the 63 KiB cascade of a 64 KiB Sum fully serial. Result: Sum256/64 KiB 41.8 → 38.7 µs; 48–128 KiB writes 10–25% faster; 1 MiB unchanged in time with 155 → 131 allocs and 14.2 → 9.6 KB per op; ≤ 1 KiB unchanged. TestEigentreeWriteEquivalence pins both scheduling paths to the chunk-at-a-time reference (which never enters the eigentree code) over every tree shape to 64 chunks plus a spread to 300, at a range of starting counters, plain and keyed — ~5,600 comparisons. Mutating a counter, a buffer advance, or a run's start on either path fails it. The group-dealing arithmetic was checked exhaustively for every power-of-two group count to 4096 × parallelism to 64. Race detector clean. Signed-off-by: apostasie <spam_blackhole@farcloser.world> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
No description provided.