Size the spill buffers from the machine, and count records honestly - #53
Closed
JamesKane wants to merge 1 commit into
Closed
Size the spill buffers from the machine, and count records honestly#53JamesKane wants to merge 1 commit into
JamesKane wants to merge 1 commit into
Conversation
…ords honestly Both of the pipeline's external sorts were sized by a constant: 512 MB for the coordinate sort, 256 MB for the revert's collator, the same on a laptop and on a 128 GB workstation. On a 30x WGS the sort spilled 688 runs, all of which the merge opens at once. It is bounded memory by design and it does work — but it is a lot of fan-in bought for no reason on a machine that was otherwise idle. navigator_resource::spill_budget is now the one rule for both: - a quarter of installed RAM. Total rather than free, because a stage whose run count depends on how many browser tabs are open cannot be reproduced from a bug report. - never below 512 MB, so no machine sorts with less than it does today. - never above 8 GB. The returns stop — 88 runs against 44 is nothing next to 688 against 88 — while the costs do not: the stable sort allocates half the buffer again as scratch, and growing the record vector holds two allocations at once. - never more than half of what is free right now, because the stable part of the rule assumes an idle machine. Memory the platform will not report reads as unknown, not as zero, and an unknown machine gets the floor — the same rule classify() follows. NAVIGATOR_SORT_MB and NAVIGATOR_REVERT_SORT_MB still win, and the job logs the figure it chose so a spill count stays explicable afterwards. Sizing from the machine also required the tally to be honest, and it was not. The sort charged a record for its name, sequence, qualities and CIGAR and not for its tag dictionary — a dozen tags on every minimap2 record at 40 bytes an entry — so a "512 MB" buffer held closer to a gigabyte. The collator's flat 64 bytes of overhead was smaller than its record struct alone. An unwritten margin inside a hand-picked constant is harmless; the same margin inside a fraction of installed RAM is how a machine ends up swapping. Both estimators now count the record's own size (RecordBuf is 176 bytes, measured), the tag dictionary, and per-allocation overhead, and a test pins the tag-entry figure so a noodles upgrade that grows Value fails there rather than silently. On a 128 GB machine this resolves to the 8 GB ceiling: roughly 84 spilled runs against 688. 994 tests pass (984 + 10); clippy and fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <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
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 #52 — it needs
navigator-resourceto exist. Rebases onto main cleanly once that merges.The constant
Both external sorts were sized by a fixed number: 512 MB for the coordinate sort, 256 MB for the revert's collator, identical on a laptop and on a 128 GB workstation. A 30x WGS spilled 688 runs, all of which the merge opens at once. Bounded memory by design, and it works — it is just a lot of fan-in bought for no reason on a machine sitting idle.
One rule, both stages
navigator_resource::spill_budget(var):Unknown memory reads as the floor rather than as zero — the rule
classifyalready follows.NAVIGATOR_SORT_MB/NAVIGATOR_REVERT_SORT_MBstill override, and the job logs the figure it chose.The part that wasn't optional
Sizing from the machine is only safe if the tally is honest, and it wasn't.
heap_bytescharged a record for its name, sequence, qualities and CIGAR — and not for its tag dictionary. Every minimap2 record carries ~12 tags at 40 bytes an entry, so a "512 MB" buffer held closer to a gigabyte. The collator's flat 64 bytes of overhead was smaller than its own record struct.That margin is harmless inside a hand-picked constant and dangerous inside a fraction of installed RAM. Both estimators now count the record's own size (
RecordBuf= 176 bytes, measured), the tag dictionary, and per-allocation overhead. A test pins the tag-entry figure so a noodles upgrade that growsValuefails there rather than silently halving the buffer's honesty.Effect
On a 128 GB machine: the 8 GB ceiling, roughly 84 spilled runs against 688. On 16 GB: a 4 GB buffer. On 8 GB with 6 free: 2 GB.
994 pass, 0 fail (984 + 10 new). Clippy clean under
-D warnings, fmt clean.Not done
🤖 Generated with Claude Code