From a5b05ade324267db66721581cd98fe575746d3d3 Mon Sep 17 00:00:00 2001 From: James Kane Date: Sun, 16 Aug 2026 07:57:41 -0500 Subject: [PATCH] feat(realign): size the spill buffers from the machine, and count records honestly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- CLAUDE.md | 2 +- .../src/postprocess/sort.rs | 53 +++++--- .../src/postprocess/tests.rs | 45 +++++++ .../navigator-analysis/src/revert/collate.rs | 6 +- crates/navigator-analysis/src/revert/mod.rs | 17 +-- .../src/revert/transform.rs | 10 +- crates/navigator-app/src/realign_job.rs | 17 ++- crates/navigator-resource/src/lib.rs | 121 ++++++++++++++++++ documents/design/realignment-module.md | 25 +++- 9 files changed, 259 insertions(+), 37 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ee45985c..6cac7c1a 100755 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,4 +100,4 @@ Shared crates (`du-domain`, `du-atproto`, `du-bio`) live in the sibling repo `.. ### Useful Environment Variables -`NAVIGATOR_ANALYSIS_THREADS`, `NAVIGATOR_BGZF_THREADS`, `NAVIGATOR_IO_SYNC_MB` (how much a multi-GB writer may leave dirty in the page cache; `0` disables the pacing), `NAVIGATOR_Y_TREE_PROVIDER` (`decodingus`/`ftdna`), `NAVIGATOR_TREE_TTL_DAYS`, `NAVIGATOR_REFGENOME_DIR`, `NAVIGATOR_TREE_DIR`, `NAVIGATOR_ANCESTRY_PANEL` / `NAVIGATOR_ANCESTRY_PCA`, `DECODINGUS_APPVIEW_URL`. +`NAVIGATOR_ANALYSIS_THREADS`, `NAVIGATOR_BGZF_THREADS`, `NAVIGATOR_IO_SYNC_MB` (how much a multi-GB writer may leave dirty in the page cache; `0` disables the pacing), `NAVIGATOR_SORT_MB` / `NAVIGATOR_REVERT_SORT_MB` (spill-buffer size in MB; both are sized from installed RAM when unset), `NAVIGATOR_Y_TREE_PROVIDER` (`decodingus`/`ftdna`), `NAVIGATOR_TREE_TTL_DAYS`, `NAVIGATOR_REFGENOME_DIR`, `NAVIGATOR_TREE_DIR`, `NAVIGATOR_ANCESTRY_PANEL` / `NAVIGATOR_ANCESTRY_PCA`, `DECODINGUS_APPVIEW_URL`. diff --git a/crates/navigator-analysis/src/postprocess/sort.rs b/crates/navigator-analysis/src/postprocess/sort.rs index f4f8787e..506409df 100644 --- a/crates/navigator-analysis/src/postprocess/sort.rs +++ b/crates/navigator-analysis/src/postprocess/sort.rs @@ -1,8 +1,13 @@ //! Coordinate-sort a BAM, on disk. //! //! Same shape as [`crate::revert::collate`] and for the same reason: a WGS alignment does not fit -//! in memory, so this fills a fixed budget, sorts it, spills a run, and k-way merges the runs at -//! the end. Peak memory is the budget plus one buffered block per run, independent of input size. +//! in memory, so this fills a budget, sorts it, spills a run, and k-way merges the runs at the end. +//! Peak memory is the budget plus one buffered block per run, independent of input size. +//! +//! The budget comes from the machine ([`navigator_resource::spill_budget`]), not from a constant. +//! It was 512 MB for everyone, which on a 30x WGS spilled **688 runs** that the merge then opened +//! at once — bounded memory by design, and a great deal of fan-in to buy on a machine with 128 GB +//! sitting idle. //! //! ## Runs are ordinary BAM files //! @@ -35,9 +40,6 @@ use crate::error::AnalysisError; /// How often the record loop asks whether it has been cancelled — same cadence as the walkers. const CANCEL_CHECK_INTERVAL: u64 = 4096; -/// Default in-memory budget before a run is spilled. -const DEFAULT_SORT_BUFFER_MB: usize = 512; - /// Tuning for [`sort_alignment`]. #[derive(Debug, Clone)] pub struct SortParams { @@ -46,14 +48,12 @@ pub struct SortParams { } impl Default for SortParams { + /// Sized from the machine, not from a constant — see [`navigator_resource::spill_budget`], which + /// also documents `NAVIGATOR_SORT_MB`. The constant this replaced was 512 MB, which spilled 688 + /// runs on a 30x WGS regardless of whether the machine had 8 GB or 128 GB to work with. fn default() -> Self { - let mb = std::env::var("NAVIGATOR_SORT_MB") - .ok() - .and_then(|s| s.parse::().ok()) - .unwrap_or(DEFAULT_SORT_BUFFER_MB) - .max(1); Self { - buffer_bytes: mb * 1024 * 1024, + buffer_bytes: navigator_resource::spill_budget("NAVIGATOR_SORT_MB") as usize, } } } @@ -322,17 +322,38 @@ fn sort_key(record: &RecordBuf) -> (u32, u64) { } } -/// Rough heap footprint, for the memory budget. The variable-length parts dominate; the constant -/// covers the record's fixed fields and allocator overhead closely enough to size a buffer by. -fn heap_bytes(record: &RecordBuf) -> usize { +/// What one record costs the buffer. +/// +/// This used to be the variable-length parts plus a flat 256, which read as a reasonable stand-in +/// for "fixed fields and allocator overhead" and was not one. It left out the tag dictionary +/// entirely, and a mapped record carries a dozen tags — `NM`, `MD`, `AS`, `ms`, `nn`, `tp`, `cm`, +/// `s1`, `s2`, `de`, `rl` from minimap2 alone — each an entry in a `Vec<(Tag, Value)>`. The buffer +/// therefore held well over its stated budget, which mattered little against a constant picked with +/// an unwritten margin and matters a great deal now that the budget is a fraction of the machine +/// (see [`navigator_resource::spill_budget`]). +/// +/// Still an estimate: it does not chase a `Value`'s own heap (a string tag's bytes) or a `Vec`'s +/// spare capacity. It is close enough to size a buffer by, and it no longer omits a whole field. +pub(super) fn heap_bytes(record: &RecordBuf) -> usize { use noodles::sam::alignment::record::Cigar as _; - record.name().map(|n| n.len()).unwrap_or(0) + // The record itself sits inline in the buffer's `Vec`, so its size is part of what a record + // costs — not something to approximate around. + std::mem::size_of::() + + record.name().map(|n| n.len()).unwrap_or(0) + record.sequence().len() + record.quality_scores().len() + record.cigar().len() * 4 - + 256 + + record.data().len() * TAG_ENTRY_BYTES + // Name, sequence, qualities, CIGAR, tags: five vectors, five allocations. + + 5 * navigator_resource::ALLOCATION_OVERHEAD } +/// Bytes one tag occupies in a record's `Vec<(Tag, Value)>`. +/// +/// Pinned by `a_tag_entry_is_not_larger_than_the_estimate_assumes`, so a noodles upgrade that grows +/// `Value` fails a test here rather than quietly halving the buffer's honesty. +pub(super) const TAG_ENTRY_BYTES: usize = 48; + /// Stamp `@HD SO:coordinate` on the header. /// /// Not cosmetic: an index is only valid for a coordinate-sorted file, and readers decide whether diff --git a/crates/navigator-analysis/src/postprocess/tests.rs b/crates/navigator-analysis/src/postprocess/tests.rs index 6e2c5ec9..5763dedf 100644 --- a/crates/navigator-analysis/src/postprocess/tests.rs +++ b/crates/navigator-analysis/src/postprocess/tests.rs @@ -116,6 +116,51 @@ fn unsorted_fixture(dir: &Path) -> (PathBuf, sam::Header, usize) { (input, hdr, total) } +// ---- the buffer estimate --------------------------------------------------- + +/// The buffer is now sized as a fraction of the machine rather than a hand-picked constant, so what +/// a record is charged against it has to be roughly true. It was not: the tag dictionary was free, +/// and a mapped record carries a dozen tags. +#[test] +fn the_buffer_estimate_counts_the_tag_dictionary() { + use noodles::sam::alignment::record::data::field::Tag; + use noodles::sam::alignment::record_buf::data::field::Value; + + let bare = record("r0", Some(0), 1); + let mut tagged = bare.clone(); + for (tag, value) in [ + (Tag::ALIGNMENT_HIT_COUNT, Value::from(1i32)), + (Tag::MISMATCHED_POSITIONS, Value::from("10")), + (Tag::ALIGNMENT_SCORE, Value::from(60i32)), + ] { + tagged.data_mut().insert(tag, value); + } + + let charged = heap_bytes(&tagged) - heap_bytes(&bare); + assert_eq!(charged, 3 * TAG_ENTRY_BYTES, "three tags should cost three entries"); +} + +/// A noodles upgrade that grows `Value` should fail here, rather than quietly making every buffer +/// hold more than its budget says. +#[test] +fn a_tag_entry_is_not_larger_than_the_estimate_assumes() { + use noodles::sam::alignment::record::data::field::Tag; + use noodles::sam::alignment::record_buf::data::field::Value; + + assert!( + std::mem::size_of::<(Tag, Value)>() <= TAG_ENTRY_BYTES, + "a tag entry is {} bytes, which the estimate does not cover", + std::mem::size_of::<(Tag, Value)>() + ); +} + +/// The record's own size is part of what it costs — it lives inline in the buffer's `Vec`. +#[test] +fn the_buffer_estimate_covers_the_record_itself() { + let empty = RecordBuf::default(); + assert!(heap_bytes(&empty) >= std::mem::size_of::()); +} + // ---- the properties that matter ------------------------------------------- /// Coordinate order, with unplaced reads at the end where SAM puts them. diff --git a/crates/navigator-analysis/src/revert/collate.rs b/crates/navigator-analysis/src/revert/collate.rs index b68f648b..38640bee 100644 --- a/crates/navigator-analysis/src/revert/collate.rs +++ b/crates/navigator-analysis/src/revert/collate.rs @@ -8,7 +8,8 @@ //! //! The property that matters is that peak memory is the budget plus one buffered block per run, //! *independent of input size*. That is what lets the same code path revert a 5 GB exome and a -//! 200 GB WGS on the same laptop, and it is why this is disk-backed rather than clever. +//! 200 GB WGS on the same laptop, and it is why this is disk-backed rather than clever. The budget +//! itself is sized from the machine — see [`navigator_resource::spill_budget`]. //! //! Runs use a plain length-prefixed binary encoding rather than a serialization framework: the //! format is written and read in this one file, it is a hot path measured in billions of records, @@ -25,7 +26,8 @@ use super::transform::{Mate, RevertedRead}; use crate::error::AnalysisError; /// Buffer size for run spill/read-back. Large enough that the merge's per-run reads stay -/// sequential, small enough that a few dozen concurrent runs don't add up to real memory. +/// sequential, small enough that the runs a WGS produces don't add up to real memory when the merge +/// holds all of them open at once. const RUN_IO_BUFFER: usize = 256 * 1024; /// Accumulates reverted reads, spilling sorted runs to scratch when the budget is reached. diff --git a/crates/navigator-analysis/src/revert/mod.rs b/crates/navigator-analysis/src/revert/mod.rs index 84da79ee..607be822 100644 --- a/crates/navigator-analysis/src/revert/mod.rs +++ b/crates/navigator-analysis/src/revert/mod.rs @@ -56,11 +56,6 @@ pub use transform::{Mate, RevertedRead}; /// reasoning as the other walkers (see [`crate::cancel`]). const CANCEL_CHECK_INTERVAL: u64 = 4096; -/// Default memory budget for the sort buffer before a run is spilled to scratch. 256 MiB holds -/// roughly a million 150 bp reads with names, which keeps the run count (and so the merge's open -/// file handles) low for a WGS input without being greedy on an 8 GB machine. -const DEFAULT_SORT_BUFFER_MB: usize = 256; - /// What to do with a **primary** record whose CIGAR contains a hard clip. /// /// Hard clipping means the aligner discarded sequence from the record, so the read cannot be fully @@ -90,14 +85,14 @@ pub struct RevertParams { } impl Default for RevertParams { + /// The collator is sized from the machine by the same rule as the coordinate sort — see + /// [`navigator_resource::spill_budget`], which also documents `NAVIGATOR_REVERT_SORT_MB`. The + /// constant this replaced was 256 MB, described as keeping the run count low for a WGS; at + /// ~340 bytes a reverted read that is a run every million reads, so a 30x WGS spilled several + /// hundred of them and the merge opened every one. fn default() -> Self { - let mb = std::env::var("NAVIGATOR_REVERT_SORT_MB") - .ok() - .and_then(|s| s.parse::().ok()) - .unwrap_or(DEFAULT_SORT_BUFFER_MB) - .max(1); Self { - sort_buffer_bytes: mb * 1024 * 1024, + sort_buffer_bytes: navigator_resource::spill_budget("NAVIGATOR_REVERT_SORT_MB") as usize, hard_clipped: HardClipPolicy::default(), prefer_original_qualities: true, } diff --git a/crates/navigator-analysis/src/revert/transform.rs b/crates/navigator-analysis/src/revert/transform.rs index 55dfbd04..27dfb274 100644 --- a/crates/navigator-analysis/src/revert/transform.rs +++ b/crates/navigator-analysis/src/revert/transform.rs @@ -64,7 +64,15 @@ impl RevertedRead { /// Rough heap footprint, for the sort's memory budget. The two `Vec`s dominate; the constant /// covers the struct itself and allocator overhead closely enough to size a buffer by. pub fn heap_bytes(&self) -> usize { - self.name.len() + self.sequence.len() + self.qualities.len() + 64 + // The read sits inline in the collator's `Vec`, so its own size counts; the three vectors + // each cost an allocation on top of their contents. The flat 64 this replaced was smaller + // than the struct alone, which was harmless against a fixed 256 MB budget and is not + // against a budget sized from the machine (see `navigator_resource::spill_budget`). + std::mem::size_of::() + + self.name.len() + + self.sequence.len() + + self.qualities.len() + + 3 * navigator_resource::ALLOCATION_OVERHEAD } } diff --git a/crates/navigator-app/src/realign_job.rs b/crates/navigator-app/src/realign_job.rs index 9cd34c23..649ca7aa 100644 --- a/crates/navigator-app/src/realign_job.rs +++ b/crates/navigator-app/src/realign_job.rs @@ -356,8 +356,10 @@ impl App { let dir = scratch.path().join("revert"); clear_stage_dir(&dir); let token = cancel.clone(); + let params = RevertParams::default(); + log_buffer("collating", params.sort_buffer_bytes); let stats = tokio::task::spawn_blocking(move || { - revert::revert_alignment(&bam, reference.as_deref(), &dir, &RevertParams::default(), &token) + revert::revert_alignment(&bam, reference.as_deref(), &dir, ¶ms, &token) }) .await .map_err(|e| AppError::Join(e.to_string()))??; @@ -466,9 +468,11 @@ impl App { // weight — and at WGS scale they are tens of GB the sort is about to want back. clear_stage_dir(&dir); let token = cancel.clone(); + let params = SortParams::default(); + log_buffer("sorting", params.buffer_bytes); stage(&sorted, async { tokio::task::spawn_blocking(move || { - postprocess::sort_alignment(&input, &out, &dir, &SortParams::default(), &token, &mut |_| {}) + postprocess::sort_alignment(&input, &out, &dir, ¶ms, &token, &mut |_| {}) }) .await .map_err(|e| AppError::Join(e.to_string()))? @@ -673,6 +677,15 @@ fn gb(bytes: u64) -> u64 { bytes / 1_000_000_000 } +/// Record the spill budget a stage is about to run with. +/// +/// Both budgets now come from the machine rather than a constant, so the run count they produce is +/// no longer inferable from the version number. Without this line, "it spilled 400 runs" in a bug +/// report is a fact with nothing to attach it to. +fn log_buffer(stage: &str, bytes: usize) { + eprintln!("realign: {stage} with a {} MB buffer", bytes / (1024 * 1024)); +} + /// Whether a job needing `needed` bytes may start given `free` bytes available. /// /// Split from the syscall so the *decision* is testable on any machine; the probe itself is not. diff --git a/crates/navigator-resource/src/lib.rs b/crates/navigator-resource/src/lib.rs index 479035a9..055b1c7e 100644 --- a/crates/navigator-resource/src/lib.rs +++ b/crates/navigator-resource/src/lib.rs @@ -150,6 +150,75 @@ impl Write for PacedFile { } } +/// The budget an external-sort stage holds in memory before spilling a run to disk. +/// +/// Both of the pipeline's spill-to-disk stages — the revert's collator and the coordinate sort — +/// were sized by a constant, 256 MB and 512 MB, chosen when nobody had run a WGS through them. +/// The measured cost of that: a 30x WGS coordinate sort spilled **688 runs**, all of which the +/// merge then opens at once. It is bounded memory by design and it does work, but on a machine with +/// 128 GB of RAM it is a lot of fan-in bought for no reason, and the constant that produced it was +/// the same on a laptop that genuinely needed it. +/// +/// So the number comes from the machine. `var` still wins when it is set — an explicit MB count is +/// the escape hatch for a run that has to be reproduced or squeezed — and the sizing is otherwise: +/// +/// - **A quarter of installed RAM.** Total rather than free, because free fluctuates with whatever +/// the user happens to have open, and a stage whose run count depends on the browser is a stage +/// whose behaviour cannot be reproduced from a bug report. +/// - **Never below 512 MB.** That is the sort's existing default, so no machine sorts with less +/// than it does today. +/// - **Never above 8 GB.** Past that the returns are gone — 88 runs against 44 is nothing next to +/// 688 against 88 — while the costs are not: the stable sort allocates half the buffer again as +/// scratch, and growing the record vector doubles its allocation while still holding the old one. +/// - **Never more than half of what is free right now.** The stable part of the rule assumes a +/// machine that is otherwise idle. When it is not, spilling an extra run is cheap and swapping +/// the buffer is not. +/// +/// Memory the platform will not report reads as zero, and zero means *unknown*, which must not be +/// read as "no memory" — the same rule [`classify`] follows. An unknown machine gets the floor. +pub fn spill_budget(var: &str) -> u64 { + if let Some(mb) = std::env::var(var).ok().and_then(|s| s.parse::().ok()) { + return mb.max(1) * 1024 * 1024; + } + let mut system = sysinfo::System::new(); + system.refresh_memory(); + budget(system.total_memory(), system.available_memory()) +} + +/// What one heap allocation costs beyond the bytes asked for: the allocator's own bookkeeping plus +/// rounding up to a size class. +/// +/// It lives beside [`spill_budget`] because the two are one contract. A budget is only as honest as +/// the tally that fills it, and a stage that counts only payload bytes will hold well over its +/// budget in real memory — which is fine against a hand-picked 512 MB constant chosen with a margin +/// nobody wrote down, and not fine against a fraction of the machine. +/// +/// Sixteen bytes is the conventional figure for the allocators on the three desktop targets. This +/// is a budget estimate rather than an audit; the point is that a record with four small vectors +/// costs meaningfully more than the sum of their lengths. +pub const ALLOCATION_OVERHEAD: usize = 16; + +/// The floor, and the answer for a machine that will not say how much memory it has. +const MIN_SPILL_BUDGET: u64 = 512 << 20; +/// The ceiling. See [`spill_budget`] for why bigger stops paying. +const MAX_SPILL_BUDGET: u64 = 8 << 30; + +/// The sizing decision, split from the probe so it is testable on any machine — the same split +/// [`classify`] makes, and for the same reason. +fn budget(total_memory: u64, available_memory: u64) -> u64 { + if total_memory == 0 { + return MIN_SPILL_BUDGET; + } + let budget = (total_memory / 4).clamp(MIN_SPILL_BUDGET, MAX_SPILL_BUDGET); + if available_memory == 0 { + return budget; + } + // The floor holds even here: a machine this short of memory would have taken 512 MB under the + // old constant anyway, so honouring the busy-machine guard past that point would be a + // regression dressed as caution. + budget.min((available_memory / 2).max(MIN_SPILL_BUDGET)) +} + /// How hard the machine is being leaned on. /// /// Bands, not a single threshold, because the interesting reading is the trend: a stage that @@ -361,6 +430,58 @@ mod tests { assert_eq!(classify(0, 0, 0), Pressure::Normal); } + /// The case the autosizing exists for: a big machine should stop spilling hundreds of runs. + #[test] + fn a_large_machine_gets_the_ceiling() { + assert_eq!(budget(128 << 30, 100 << 30), MAX_SPILL_BUDGET); + } + + #[test] + fn an_ordinary_machine_gets_a_quarter_of_it() { + assert_eq!(budget(16 << 30, 12 << 30), 4 << 30); + } + + /// A machine whose memory is already spoken for gets a smaller buffer, because an extra spilled + /// run costs a file and swapping the buffer costs the run. + #[test] + fn a_busy_machine_is_held_to_half_of_what_is_free() { + assert_eq!(budget(64 << 30, 6 << 30), 3 << 30); + } + + /// Never below what the sort used before any of this existed. + #[test] + fn a_small_or_busy_machine_never_goes_under_the_old_default() { + assert_eq!(budget(2 << 30, 2 << 30), MIN_SPILL_BUDGET); + assert_eq!(budget(64 << 30, 100 << 20), MIN_SPILL_BUDGET); + } + + /// Unknown is not zero. A platform that will not report memory must not be sized as if it had + /// none — and must not be sized as if it had plenty either. + #[test] + fn unknown_memory_gets_the_floor() { + assert_eq!(budget(0, 0), MIN_SPILL_BUDGET); + assert_eq!(budget(64 << 30, 0), MAX_SPILL_BUDGET.min(16 << 30)); + } + + /// The escape hatch has to win, or a run cannot be reproduced on a different machine. + #[test] + fn an_explicit_override_beats_the_machine() { + let var = "NAVIGATOR_TEST_SPILL_MB_OVERRIDE"; + std::env::set_var(var, "7"); + assert_eq!(spill_budget(var), 7 * 1024 * 1024); + std::env::remove_var(var); + } + + /// A nonsense override must not produce a zero-byte budget, which would spill one run per + /// record. + #[test] + fn a_zero_override_is_floored_at_one_megabyte() { + let var = "NAVIGATOR_TEST_SPILL_MB_ZERO"; + std::env::set_var(var, "0"); + assert_eq!(spill_budget(var), 1024 * 1024); + std::env::remove_var(var); + } + #[test] fn written_bytes_accumulate() { let before = bytes_written(); diff --git a/documents/design/realignment-module.md b/documents/design/realignment-module.md index c08ce6dd..48b0b1eb 100644 --- a/documents/design/realignment-module.md +++ b/documents/design/realignment-module.md @@ -640,10 +640,27 @@ as a page-cache promise — which matters most for `mapped.bam`, since its BGZF precisely what a resumed run reads to decide whether it can trust the file, and the .mmi index, whose atomic rename otherwise publishes contents the disk has not acknowledged. -The sort buffer is worth revisiting separately: at the default 512 MB it spilled **688 runs**, which -the merge then opens at once. That is bounded memory by design and it works, but on a 128 GB machine -it is a lot of fan-in bought for no reason. `NAVIGATOR_SORT_MB` already exists; sizing its default -from installed RAM is not yet done. +**The spill budgets now come from the machine.** At the fixed 512 MB the sort spilled **688 runs**, +which the merge opens at once — bounded memory by design, and a lot of fan-in to buy on a 128 GB +machine that was otherwise idle. The revert's collator had the same shape at 256 MB. Both now call +`navigator_resource::spill_budget`: a quarter of installed RAM, never below the old 512 MB default, +never above 8 GB, and never more than half of what is free at the moment the stage starts. +`NAVIGATOR_SORT_MB` and `NAVIGATOR_REVERT_SORT_MB` still override it, and the job logs the figure it +chose so a run's spill count stays explicable afterwards. + +Total RAM rather than free RAM for the stable part of the rule, deliberately: a stage whose run +count depends on how many browser tabs were open is a stage whose behaviour cannot be reproduced +from a bug report. The ceiling is there because 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. + +Sizing from the machine also required the tally to be honest, which 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. 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, +its 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. ## Phase 5 result — WGS229 end to end (2026-08-14)