Claude/extsort perf batching - #15
Merged
Merged
Conversation
Push wrapped each element in an item, boxed it for container/heap and stored a pointer to a copy: two allocations per element. It then ran heap.Fix on the index of the original, which was always 0, so it re-sifted the root for nothing. Keep the elements in a slice and sift them directly with the algorithm of container/heap, so Push and Pop no longer allocate and the comparisons are no longer interface calls. Pushing and popping through a 64-element queue drops from 66.2 ns and 2 allocations to 23.9 ns and none (BenchmarkPushPop); replacing the top of a 64-way merge drops from 38.9 ns to 22.2 ns (BenchmarkPeekUpdate). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every read was a select on the data channel and ctx.Done(), and a select locks both channels, so readers of a busy stream contended on the context's channel for every value. Try a non-blocking receive first and fall back to the select only when the stream has nothing ready. ctx is then checked every 1024 reads, starting with the first, so a diff with values always ready still sees a cancellation, and a diff whose ctx is already cancelled returns before reading anything (the select used to pick a case at random). Diffing two streams of 1M ints drops from 179 ms to 70 ms (BenchmarkDiffOrdered). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every sorter created the pool, and nothing ever took a slice from it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each merge worker sent its records to the final merge one per channel operation, every send a select on the same ctx.Done() channel as all the other workers, so the handoffs cost more than the merge: most CPU time went to the scheduler and to channel locks. Workers now send batches of 1024 records, with up to two queued per worker, and the final merge hands used-up batches back to the worker for reuse. Errors, cancellation and panic recovery take the same paths as before, and a worker checks ctx as it sends each batch, so it stops within one batch of a cancel. Sorting 1M ints with Generic drops from 350 ms to 134 ms (BenchmarkGenericVarintInts) and 1M strings from 377 ms to 183 ms (BenchmarkLegacyStringSort/size_1000000), and 2,000 chunks merge in 52 ms instead of 111 ms (BenchmarkSortManyChunks). New tests make a read error partway into a chunk, a compareFunc panic and a cancel each happen mid-merge, and check that the error reaches the error channel, the temp file is closed and no sorter goroutine is left running. Another covers record counts around the batch size. trackedTemp gains readErrAfter to fail a section partway, and the final merge panic test feeds the final merge through batch streams. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
buildChunks read every record with a select on the input and on ctx.Done(), which locks both channels. Try a non-blocking receive first and fall back to the select only when the input has nothing ready. A steady input then costs one channel operation per record, and ctx is checked every 1024 records so it still sees a cancellation. Sorting 1M ints fed by a goroutine drops from 134 ms to 109 ms (BenchmarkGenericVarintInts). The output send keeps its select: a non-blocking send first made the merge slower when measured. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The slice pool gave every chunk a slice of capacity ChunkSize, so sorting 10 ints with the default config allocated 8 MB. A chunk now starts at 1024 records and doubles, to exactly ChunkSize. Once a chunk has filled, the input spans several chunks, so later chunks take their full size at once instead of growing. Sorting 10 records drops from 47.7 µs and 7.6 MiB to 10.0 µs and 23 KiB (BenchmarkSortTenRecords). Large sorts pay for growing the first chunk once: BenchmarkGenericVarintInts allocates 3.5% more, in the same time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Strings and Ordered allocated a slice to encode every record and another to read it back in the merge. Their codecs now append each record to one buffer reused across the chunk, written together with its length header in a single Write, and the merge reads all the records of a chunk into one reused buffer. That is only safe because these fromBytes functions never keep the slice they are given; Generic keeps allocating, since a user's fromBytes may. The scratch-buffer pool for the length header becomes a stack array. Sorting 1M ints with Ordered drops from 45.4 MiB and 3.0M allocations to 6.8 MiB and 277, and from 112 ms to 104 ms (BenchmarkOrderedInts). 1M strings take 34.9 MiB and 1M allocations instead of 51.4 MiB and 3M (BenchmarkLegacyStringSort/size_1000000). Co-Authored-By: Claude Opus 5.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.