fix(bench): hot loops join vm.json, a broken snapshot fails, and memory keeps every peak past a 4 KB floor

Author: dylan-sutton-chavez

CONTRIBUTING.md

@@ -61,14 +61,14 @@ deno lint js/
 
 ### Bench
 
-`cargo run -p bench --profile cli` runs every case of `vm.json` on the `cargo wasm-cli` build, and stops when that build is older than `src/`. It counts the WebAssembly instructions each case executes and prices each at 0.82 ns, the 822756 gas that `wasm_regular_op_cost` sets in `core/parameters/res/runtime_configs/parameters.yaml` of nearcore, at the 1 ms per Tgas its gas estimator budgets, so `bench/.snapshot` keeps reference seconds that come out the same on every machine. Each case keeps them beside its memory peak in MB, by the model the memory limit counts, as `[seconds, MB]`.
+`cargo run -p bench --profile cli` runs every case of `vm.json` on the `cargo wasm-cli` build, and stops when that build is older than `src/`. It counts the WebAssembly instructions each case executes and prices each at 0.82 ns, the 822756 gas that `wasm_regular_op_cost` sets in `core/parameters/res/runtime_configs/parameters.yaml` of nearcore, at the 1 ms per Tgas its gas estimator budgets, so `bench/.snapshot` keeps reference seconds that come out the same on every machine. Each case keeps them beside its memory peak in MB, by the model the memory limit counts, as `[seconds, MB]`. The peak includes garbage not yet collected, so a change to when the collector runs moves it too.
 
 The Bench job fails a pull request when
 
 - the snapshot was taken with another Rust,
 - a case is missing from it, or an entry has no case,
 - the geometric mean of time or memory moves past its threshold, either way,
-- a single case moves past its own threshold in either.
+- a single case moves past its own threshold in either, and in memory also by more than `memory_floor` MB.
 
 Every run prints how the cases already in the snapshot moved, even when another rule fails. `--update` reports the change and takes the snapshot again, and a faster engine takes it too, so the next change is measured from where the code stands.
 

bench/.snapshot

@@ -22,12 +22,15 @@ struct Snapshot {
     rustc: String,
     threshold: f64,
     case_threshold: f64,
+    // MB a case may move in memory before its own threshold applies, so one small object is not a finding.
+    memory_floor: f64,
     cases: BTreeMap<String, (f64, f64)>,
 }
 
 // How a report words one measure, so time and memory are held to the same rules.
 struct Measure {
     pick: fn((f64, f64)) -> f64,
+    floor: fn(&Snapshot) -> f64,
     unit: &'static str,
     places: usize,
     grew: &'static str,
@@ -38,8 +41,8 @@ struct Measure {
     total: (&'static str, &'static str),
 }
 
-const TIME: Measure = Measure { pick: |c| c.0, unit: "s", places: 9, grew: "got slower", shrank: "got faster", most_grown: "Most slowed", most_shrunk: "Biggest gains", own: "", total: ("runs in", "of reference time") };
-const MEMORY: Measure = Measure { pick: |c| c.1, unit: "MB", places: 6, grew: "holds more memory", shrank: "holds less memory", most_grown: "Most grown", most_shrunk: "Biggest drops", own: " in memory", total: ("peaks at", "summed over its cases") };
+const TIME: Measure = Measure { pick: |c| c.0, floor: |_| 0.0, unit: "s", places: 9, grew: "got slower", shrank: "got faster", most_grown: "Most slowed", most_shrunk: "Biggest gains", own: "", total: ("runs in", "of reference time") };
+const MEMORY: Measure = Measure { pick: |c| c.1, floor: |s| s.memory_floor, unit: "MB", places: 6, grew: "holds more memory", shrank: "holds less memory", most_grown: "Most grown", most_shrunk: "Biggest drops", own: " in memory", total: ("peaks at", "summed over its cases") };
 
 fn main() {
     let update = std::env::args().any(|a| a == "--update");
@@ -60,17 +63,28 @@ fn main() {
     let ran: Vec<(String, (u64, u64))> = cases.iter().map(|case| (key(case), run(&engine, &module, case))).collect();
     let measured = ran.iter().map(|(k, (ops, peak))| (k.clone(), ((*ops as f64 * SECONDS_PER_INSTRUCTION * 1e9).round() / 1e9, (*peak as f64 / MB * 1e6).round() / 1e6))).collect();
     let rustc = Command::new("rustc").arg("--version").current_dir(ROOT).output().map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string()).unwrap_or_default();
-    let now = Snapshot { rustc, threshold: 0.005, case_threshold: 0.05, cases: measured };
+    let now = Snapshot { rustc, threshold: 0.005, case_threshold: 0.05, memory_floor: 0.004, cases: measured };
     let sources: BTreeMap<String, &str> = cases.iter().map(|c| (key(c), c["src"].as_str().unwrap_or(""))).collect();
 
-    let last = std::fs::read_to_string(SNAPSHOT).ok().and_then(|s| serde_json::from_str::<Snapshot>(&s).ok());
+    // Only a missing snapshot is taken without comparing, one that does not parse fails unless --update replaces it.
+    let last = match std::fs::read_to_string(SNAPSHOT) {
+        Err(_) => None,
+        Ok(text) => match serde_json::from_str::<Snapshot>(&text) {
+            Ok(snapshot) => Some(snapshot),
+            Err(_) if update => None,
+            Err(e) => {
+                annotate("error", &format!("bench/.snapshot does not parse, {e}, take it again with --update."));
+                std::process::exit(1);
+            }
+        },
+    };
     let failures = last.as_ref().map(|l| check(l, &now, &sources)).unwrap_or_default();
     for failure in &failures {
         annotate("error", failure);
     }
     if update || last.is_none() {
-        let thresholds = last.map_or((now.threshold, now.case_threshold), |l| (l.threshold, l.case_threshold));
-        write(&Snapshot { threshold: thresholds.0, case_threshold: thresholds.1, ..now });
+        let (threshold, case_threshold, memory_floor) = last.map_or((now.threshold, now.case_threshold, now.memory_floor), |l| (l.threshold, l.case_threshold, l.memory_floor));
+        write(&Snapshot { threshold, case_threshold, memory_floor, ..now });
         return println!("  snapshot written");
     }
     if !failures.is_empty() {
@@ -98,9 +112,10 @@ fn check(last: &Snapshot, now: &Snapshot, sources: &BTreeMap<String, &str>) -> V
 /* One measure against the snapshot, its geometric mean held to the band and each case to its own threshold. */
 fn compare(m: &Measure, last: &Snapshot, now: &Snapshot, sources: &BTreeMap<String, &str>) -> Vec<String> {
     let mut failures = Vec::new();
-    let unit = m.unit;
+    let (unit, floor) = (m.unit, (m.floor)(last));
+    let all: Vec<(&String, f64, f64)> = now.cases.iter().filter_map(|(k, &n)| last.cases.get(k).map(|&l| (k, (m.pick)(l), (m.pick)(n)))).collect();
     // Every case weighs the same in a geometric mean, so no single long case decides it.
-    let paired: Vec<(&String, f64, f64)> = now.cases.iter().filter_map(|(k, &n)| last.cases.get(k).map(|&l| (k, (m.pick)(l), (m.pick)(n)))).filter(|&(_, l, n)| l > 0.0 && n > 0.0).collect();
+    let paired: Vec<(&String, f64, f64)> = all.iter().copied().filter(|&(_, l, n)| l > 0.0 && n > 0.0).collect();
     if paired.is_empty() {
         return failures;
     }
@@ -116,11 +131,14 @@ fn compare(m: &Measure, last: &Snapshot, now: &Snapshot, sources: &BTreeMap<Stri
     } else if change < -last.threshold {
         failures.push(format!("vm.json {}, {total:.3} {unit} against {before:.3} {unit}, {:+.2}%. The snapshot no longer matches the code, run --update and commit it with the change. {}{}", m.shrank, change * 100.0, m.most_shrunk, list(&moved(false))));
     }
-    let mut apart: Vec<&(&String, f64, f64)> = paired.iter().filter(|(_, l, n)| (n / l - 1.0).abs() > last.case_threshold).collect();
-    apart.sort_by(|a, b| (b.2 / b.1).ln().abs().total_cmp(&(a.2 / a.1).ln().abs()));
+    // A case at 0 has no ratio, so leaving 0 or reaching it moves it on its own, ahead of the rest.
+    let by = |&(_, l, n): &(&String, f64, f64)| if l > 0.0 && n > 0.0 { (n / l).ln().abs() } else { f64::INFINITY };
+    let mut apart: Vec<(&String, f64, f64)> = all.iter().copied().filter(|&(_, l, n)| (n - l).abs() > floor && (l == 0.0 || n == 0.0 || (n / l - 1.0).abs() > last.case_threshold)).collect();
+    apart.sort_by(|a, b| by(b).total_cmp(&by(a)));
     if !apart.is_empty() {
-        let lines: Vec<String> = apart.iter().map(|&&(k, l, n)| moved_line(m, sources, k, l, n)).collect();
-        failures.push(format!("{} cases moved past ±{:.0}% on their own{}.{}", apart.len(), last.case_threshold * 100.0, m.own, list(&lines)));
+        let lines: Vec<String> = apart.iter().map(|&(k, l, n)| moved_line(m, sources, k, l, n)).collect();
+        let past = if floor > 0.0 { format!("±{:.0}% and {floor} {unit}", last.case_threshold * 100.0) } else { format!("±{:.0}%", last.case_threshold * 100.0) };
+        failures.push(format!("{} cases moved past {past} on their own{}.{}", apart.len(), m.own, list(&lines)));
     }
 
     // Printed even when a finding fails, so adding cases never hides how the existing ones moved.
@@ -130,7 +148,8 @@ fn compare(m: &Measure, last: &Snapshot, now: &Snapshot, sources: &BTreeMap<Stri
 
 fn moved_line(m: &Measure, sources: &BTreeMap<String, &str>, key: &str, last: f64, now: f64) -> String {
     let (unit, places) = (m.unit, m.places);
-    format!("{} {last:.places$} {unit} to {now:.places$} {unit} ({:+.1}%)", source(sources, key), (now / last - 1.0) * 100.0)
+    let change = if last > 0.0 { format!("{:+.1}%", (now / last - 1.0) * 100.0) } else { String::from("from 0") };
+    format!("{} {last:.places$} {unit} to {now:.places$} {unit} ({change})", source(sources, key))
 }
 
 fn source(sources: &BTreeMap<String, &str>, key: &str) -> String {
@@ -209,8 +228,8 @@ fn write(snapshot: &Snapshot) {
     let (t, m) = (TIME.places, MEMORY.places);
     let cases: Vec<String> = snapshot.cases.iter().map(|(k, (s, mb))| format!("    \"{k}\": [{s:.t$}, {mb:.m$}]")).collect();
     let json = format!(
-        "{{\n  \"rustc\": {:?},\n  \"threshold\": {},\n  \"case_threshold\": {},\n  \"cases\": {{\n{}\n  }}\n}}\n",
-        snapshot.rustc, snapshot.threshold, snapshot.case_threshold, cases.join(",\n")
+        "{{\n  \"rustc\": {:?},\n  \"threshold\": {},\n  \"case_threshold\": {},\n  \"memory_floor\": {},\n  \"cases\": {{\n{}\n  }}\n}}\n",
+        snapshot.rustc, snapshot.threshold, snapshot.case_threshold, snapshot.memory_floor, cases.join(",\n")
     );
     std::fs::write(SNAPSHOT, json).expect("writing bench/.snapshot");
 }

bench/src/main.rs

@@ -446,7 +446,7 @@ Distinct from the sealed plugin imports above, these are exports on `compiler.wa
 |---|---|---|
 | `set_limits` | `(memory: u64, ops: u64)` | Caps for the next `run_start` or `repl_eval`, memory in bytes. A zero field keeps the sandbox value. `restore_state` keeps the limits embedded in the blob. |
 | `set_entry` | `(ptr: *const u8, len: u32)` | Name the script the next run starts from. Tracebacks name its frame after it, and its directory roots the walk and the relative imports of the program. An empty entry, or a directory ending in `/`, runs from that directory and renders its frame as `<input>`. |
-| `memory_peak` | `() -> u64` | The most the last run held at once, in bytes, by the memory model the limit counts. |
+| `memory_peak` | `() -> u64` | The most the last run held at once, in bytes, by the memory model the limit counts, garbage not yet collected included. A run that fails to compile reads 0. |
 | `save_state` | `() -> i64` | Serialise the parked run into the out buffer. Returns the blob length, or `-1` when nothing is parked. |
 | `restore_state` | `(ptr: *const u8, len: u32) -> u32` | Boot a VM from the blob at `ptr` and overlay its state. Returns the same packed status word as `run_start`. |
 | `state_globals` | `() -> u32` | Write the parked run's module-level bindings as JSON into the out buffer. Returns its byte length. |

docs/03-reference/06-abi.mdx

@@ -623,8 +623,8 @@ pub struct HeapPool {
     alloc_count: Cell<usize>,
     // What the slots hold by the memory model, garbage since the last sweep included.
     bytes: Cell<usize>,
-    // The highest count before a sweep, where it stands tallest since only a sweep lowers it.
-    peak: usize,
+    // The highest count, kept at each place it goes down, so no high point is missed.
+    peak: Cell<usize>,
     limit: usize,
     // The first collection whose running count missed the recount, as the two totals.
     #[cfg(feature = "memcheck")]
@@ -668,7 +668,7 @@ impl HeapPool {
             gc_threshold: 512,
             alloc_count: Cell::new(0),
             bytes: Cell::new(0),
-            peak: 0,
+            peak: Cell::new(0),
             limit,
             #[cfg(feature = "memcheck")]
             drift: None,
@@ -781,12 +781,16 @@ impl HeapPool {
         let out = f(c);
         let after = c.bytes();
         if after != before {
+            if after < before { self.keep_peak(); }
             self.bytes.set(self.bytes.get().saturating_sub(before));
             self.charge(after);
         }
         out
     }
 
+    // Remembers the count before it goes down.
+    fn keep_peak(&self) { self.peak.set(self.peak.get().max(self.bytes.get())); }
+
     /* Charges what an object took, past the limit the next safe point collects. */
     #[inline]
     pub fn charge(&self, bytes: usize) {
@@ -800,8 +804,8 @@ impl HeapPool {
     /* What the slots hold by the memory model, garbage since the last sweep included. */
     pub fn bytes(&self) -> usize { self.bytes.get() }
 
-    /* The most the slots held at once, garbage included, the count now or before any sweep. */
-    pub fn peak(&self) -> usize { self.peak.max(self.bytes.get()) }
+    /* The most the slots held at once, garbage included, the count now or the highest it fell from. */
+    pub fn peak(&self) -> usize { self.peak.get().max(self.bytes.get()) }
 
     /* The same total counted again from every occupied slot, which the running count must always equal. */
     fn recount(&self) -> usize {
@@ -838,7 +842,7 @@ impl HeapPool {
     }
 
     pub fn sweep(&mut self) {
-        self.peak = self.peak.max(self.bytes.get());
+        self.keep_peak();
         // The survivors are counted again, so a sweep leaves the running count exact.
         let mut kept = 0;
         for idx in 0..self.slots.len() {
@@ -888,6 +892,7 @@ impl HeapPool {
             self.live += 1;
             self.intern_insert(idx as u32);
         }
+        self.keep_peak();
         self.bytes.set(self.recount());
         self.gc_threshold = (self.live * 2).max(512);
         self.alloc_limit = 4096;
@@ -898,6 +903,7 @@ impl HeapPool {
     /* Swap a live slot's object during restore. */
     pub(crate) fn replace_obj(&mut self, idx: u32, obj: HeapObj) {
         let before = self.slots[idx as usize].obj.as_ref().map_or(0, footprint);
+        self.keep_peak();
         self.bytes.set((self.bytes.get() + footprint(&obj)).saturating_sub(before));
         self.slots[idx as usize] = HeapSlot::new(Some(obj));
     }

src/value/mod.rs

@@ -116,11 +116,12 @@ impl Slot {
         }
     }
 
-    /* Drops the run and REPL interpreters, then the chunks they borrowed. */
+    /* Drops the run and REPL interpreters and the last memory peak, then the chunks they borrowed. */
     pub fn clear_run(&mut self) {
         self.paused_run = None;
         self.repl_vm = None;
         self.repl_mode = false;
+        self.memory_peak = 0;
         self.chunks.clear();
     }
 }