From 460b7201ba62ea48835407640030774d84fba961 Mon Sep 17 00:00:00 2001 From: Transaction Science Date: Fri, 16 Oct 2026 04:58:14 -0400 Subject: [PATCH] jouleclaw-energy: another process's GPU client going back is not this pid's problem `a_gpu_span_is_a_sole_users_rail_or_an_exact_zero_or_no_figure` fails intermittently on the macOS energy job with no client's time went back in the oracle's window `oracle_times` walks every client in the second reading and gives up the whole result if any one of their counters went back: let d = ns.checked_sub(a.clients.get(id).map_or(0, |c| c.1))?; all += d; if *p == pid { me += d; } It has to, because its `all` must be exact to compare against a share. But two of its three callers want only one pid's own time -- both read `let (child_ns, _, _) = ...` and drop `all` -- so a client of a wholly unrelated process being destroyed, or its id reused, takes away a figure that never depended on it. The GPU is shared with whatever else the host runs, which is precisely when that happens. `oracle_pid_time` totals the named pid's clients and no others, and those two callers use it. The strict reader stays where `all` is actually used. Deterministic test rather than waiting for a host client to vanish: it builds the two readings, with this pid's client advancing 5 ms while another process's goes back, and pins that `oracle_times` gives up where `oracle_pid_time` does not. It also pins the two cases that must NOT become lenient -- this pid's own client going back is still no figure, and a pid with no clients is an exact zero rather than unavailable. Verified on the GPU host, with the feature the file is gated behind: `--features ioreport` (without it the target is 0 tests and cargo still exits 0). Baseline passed=1; with `oracle_pid_time` mutated back to totalling every client the new test fails, so it discriminates rather than decorates. Co-Authored-By: Claude Opus 5 --- .../jouleclaw-energy/tests/process_gpu.rs | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-energy/tests/process_gpu.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-energy/tests/process_gpu.rs index 4f38760cc..0a033949a 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-energy/tests/process_gpu.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-energy/tests/process_gpu.rs @@ -282,6 +282,25 @@ fn oracle(rail: &OracleRail) -> Oracle { /// the clients in the second (one new there counting its whole time), and the /// IDs of the clients it saw at the first and not the second. `None` if a /// client's time went back. +/// The GPU time `pid`'s own clients ran across the window, and nothing else. +/// +/// [`oracle_times`] totals *every* client and gives up if any one of their +/// counters went back, because its `all` must be exact to compare against a +/// share. A caller that only wants one pid's own time does not need that: a +/// client of some other process being destroyed, or its id reused, says nothing +/// about this pid's clients and must not take the figure away. Only this pid's +/// own clients going back makes it unavailable. +fn oracle_pid_time(a: &Oracle, b: &Oracle, pid: i32) -> Option { + let mut me = 0u64; + for (id, (p, ns)) in &b.clients { + if *p != pid { + continue; + } + me += ns.checked_sub(a.clients.get(id).map_or(0, |c| c.1))?; + } + Some(me) +} + fn oracle_times(a: &Oracle, b: &Oracle, pid: i32) -> Option<(u64, u64, Vec)> { let (mut me, mut all) = (0u64, 0u64); for (id, (p, ns)) in &b.clients { @@ -429,6 +448,36 @@ fn assert_exact_zero(what: &str, r: &Result) { assert_eq!((g.uj, g.self_ns, g.rail_uj, g.all_ns, g.provenance()), (0, 0, None, None, Provenance::HwShunt), "{what}"); } +/// A client of some *other* process going back must not take this pid's own GPU +/// figure away. [`oracle_times`] gives up on it, because its `all` has to be +/// exact to compare against a share; [`oracle_pid_time`] does not, because it +/// totals this pid's clients and no others. Deterministic: it builds the two +/// readings rather than waiting for a client to be destroyed on the host. +#[test] +fn another_process_client_going_back_leaves_this_pids_gpu_time_readable() { + const ME: i32 = 4242; + const OTHER: i32 = 99; + let at = Instant::now(); + let mk = |clients: &[(u64, i32, u64)]| Oracle { + rail_nj: 0, + rail_at: at, + clients: clients.iter().map(|(id, pid, ns)| (*id, (*pid, *ns))).collect(), + clients_from: at, + clients_to: at, + }; + // This pid's client advances 5 ms; another process's client goes back. + let a = mk(&[(1, ME, 1_000_000), (2, OTHER, 9_000_000)]); + let b = mk(&[(1, ME, 6_000_000), (2, OTHER, 1_000_000)]); + assert_eq!(oracle_times(&a, &b, ME), None, "oracle_times totals every client, so one going back ends it"); + assert_eq!(oracle_pid_time(&a, &b, ME), Some(5_000_000), "this pid's own time is still readable"); + // This pid's own client going back is still no figure. + let c = mk(&[(1, ME, 500_000), (2, OTHER, 9_000_000)]); + assert_eq!(oracle_pid_time(&a, &c, ME), None, "this pid's own client going back makes it unavailable"); + // No client of this pid at all is zero, not unavailable. + let d = mk(&[(2, OTHER, 9_000_000)]); + assert_eq!(oracle_pid_time(&d, &d, ME), Some(0), "a pid with no clients ran no GPU time"); +} + #[test] fn a_gpu_span_is_a_sole_users_rail_or_an_exact_zero_or_no_figure() { if let Ok(secs) = std::env::var(CHILD_ENV) { @@ -521,7 +570,7 @@ fn a_gpu_span_is_a_sole_users_rail_or_an_exact_zero_or_no_figure() { // a. No GPU work here beside it: still an exact zero, while the oracle // sees the child's GPU time in the window. let (s, o0, o1, e) = bracketed(&counter, &rail, || std::thread::sleep(Duration::from_millis(400))); - let (child_ns, _, _) = oracle_times(&o0, &o1, child_pid).expect("no client's time went back in the oracle's window"); + let child_ns = oracle_pid_time(&o0, &o1, child_pid).expect("the child's own GPU time went back in the oracle's window"); assert_exact_zero(&format!("400 ms idle beside a child that ran {:.1} ms on the GPU", ms(child_ns)), &counter.share(&s, &e)); assert!(child_ns >= 20_000_000, "the child did not contend: {} ms", ms(child_ns)); // b. GPU work here beside it: `GpuShared`, at the oracle's times. @@ -534,7 +583,7 @@ fn a_gpu_span_is_a_sole_users_rail_or_an_exact_zero_or_no_figure() { line("1 s GPU burn beside a burning child", &r); let Err(EnergyError::GpuShared { self_ns, all_ns }) = r else { panic!("a shared GPU gave {r:?}") }; agrees_with_oracle("1 s GPU burn beside a burning child", self_ns, all_ns, &s, &e, &o0, &o1); - let (child_ns, _, _) = oracle_times(&o0, &o1, child_pid).expect("no client's time went back in the oracle's window"); + let child_ns = oracle_pid_time(&o0, &o1, child_pid).expect("the child's own GPU time went back in the oracle's window"); println!("process-gpu: oracle: the child ran {:.1} ms on the GPU in that window", ms(child_ns)); assert!(child_ns >= 20_000_000 && self_ns > 0, "the child did not contend: {} ms", ms(child_ns)); assert!(u128::from(self_ns) * 100 < u128::from(all_ns) * 99); -- 2.54.0 (Apple Git-157)