From acacb4c1a1bd6e3f75bc7e287413b4f3436054b0 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Fri, 14 Aug 2026 02:19:57 +0900 Subject: [PATCH 1/2] jit: drain the deferred counter decay before every table access, and apply every elapsed interval counter.py:104-121 calls `decay_all_counters()` inside the 32nd minor collection. pyre defers it, and the deferral diverged in two ways. `tick` applied one decay however many 32-collection intervals had elapsed; it now applies one per interval, stepping the generation forward one at a time so each step rounds back to f32 the way `decay_all_counters` does. `change_current_fraction`, `reset`, `reset_all` and `set_decay` did not drain at all, so a pending decay landed on values written after the collection that scheduled it: `_trace_next_iteration` (warmstate.py:617-619) writes 0.98 and the next `tick` turned it into 0.98 * 0.96. All four now drain first, and `set_decay` drains before it replaces the multiplier. `would_tick_fire` takes `&self` and answers from a decay-adjusted read, stepping the same number of generations so it agrees with the `tick` that would drain them. Assisted-by: Claude --- majit/majit-trace/src/counter.rs | 116 ++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 16 deletions(-) diff --git a/majit/majit-trace/src/counter.rs b/majit/majit-trace/src/counter.rs index 084eb93c845..6ed277bbc73 100644 --- a/majit/majit-trace/src/counter.rs +++ b/majit/majit-trace/src/counter.rs @@ -154,12 +154,25 @@ impl JitCounter { /// TODO: no RPython counterpart. Read-only peek /// used by warmstate's cold fast path to avoid GreenKey allocation. pub fn would_tick_fire(&self, hash: u64, increment: f64) -> bool { + let elapsed = DECAY_GENERATION + .load(Ordering::Relaxed) + .wrapping_sub(self.last_decay_generation); let index = self._get_index(hash); let subhash = Self::_get_subhash(hash); let entry = &self.timetable[index]; for i in 0..ASSOCIATIVITY { if entry.subhashes[i] == subhash { - return entry.times[i] as f64 + increment >= 1.0; + // This predicate is &self, so decay-adjust the read instead of + // mutating the table to apply the pending generations. Step + // through them one at a time rather than raising the multiplier + // to `elapsed`: `decay_all_counters` rounds back to f32 after + // every step, and this answer has to be the one a `tick` would + // give once it drains the same generations. + let mut time = entry.times[i]; + for _ in 0..elapsed { + time = (time as f64 * self.decay_by_mult) as f32; + } + return time as f64 + increment >= 1.0; } } increment >= 1.0 @@ -169,24 +182,16 @@ impl JitCounter { #[inline(always)] pub fn tick(&mut self, hash: u64, increment: f64) -> bool { // counter.py:104-121 applies the decay synchronously inside the minor - // collection. pyre defers it to the next tick because the metainterp - // owns the counter table, and reaching it from inside the collector - // would re-enter a borrow the GC does not hold. Counters are only read - // at tick time. + // collection. pyre defers it to the next table access because the + // metainterp owns the counter table, and reaching it from inside the + // collector would re-enter a borrow the GC does not hold. // // Each JitCounter keeps its own last-seen generation because JIT_DRIVER // in eval.rs is thread-local, giving each mutator thread its own counter. - // This still saturates: if two or more 32-collection intervals elapse - // between ticks, counter.py has applied that many decays while this - // applies one. Carrying the elapsed count would match it, but would move - // inline_chain_depth_typeflip's recorded jit-stats (bridges_compiled - // 19 -> 18, guard_failures 3820 -> 3681); left for a change that can - // re-record them on every platform. - let generation = DECAY_GENERATION.load(Ordering::Relaxed); - if generation != self.last_decay_generation { - self.last_decay_generation = generation; - self.decay_all_counters(); - } + // Every elapsed interval is applied before every mutating table access; + // would_tick_fire decay-adjusts its read. A value written after a + // collection is therefore not retro-decayed. + self.apply_pending_decay(); let index = self._get_index(hash); let subhash = Self::_get_subhash(hash); @@ -213,6 +218,8 @@ impl JitCounter { /// counter.py:204-230 change_current_fraction(hash, new_fraction) pub fn change_current_fraction(&mut self, hash: u64, new_fraction: f64) { + self.apply_pending_decay(); + let index = self._get_index(hash); let subhash = Self::_get_subhash(hash); let entry = &mut self.timetable[index]; @@ -232,6 +239,8 @@ impl JitCounter { /// counter.py:232-237 reset(hash) pub fn reset(&mut self, hash: u64) { + self.apply_pending_decay(); + let index = self._get_index(hash); let subhash = Self::_get_subhash(hash); let entry = &mut self.timetable[index]; @@ -245,6 +254,8 @@ impl JitCounter { /// TODO: no RPython equivalent. /// Zero all timetable entries. pub fn reset_all(&mut self) { + self.apply_pending_decay(); + for entry in &mut self.timetable { *entry = Entry::default(); } @@ -252,6 +263,8 @@ impl JitCounter { /// counter.py:258-264 set_decay(decay) pub fn set_decay(&mut self, decay: i32) { + self.apply_pending_decay(); + let clamped = decay.clamp(0, 1000); self.decay_by_mult = 1.0_f64 - (clamped as f64 * 0.001); } @@ -265,6 +278,18 @@ impl JitCounter { } } } + + /// Apply every 32-collection interval that elapsed since this counter + /// last looked. counter.py:104-121 decays inside the collection, so a + /// pending decay must land before anything reads or writes the table — + /// otherwise it would decay values written after the collection. + fn apply_pending_decay(&mut self) { + let generation = DECAY_GENERATION.load(Ordering::Relaxed); + while self.last_decay_generation != generation { + self.last_decay_generation = self.last_decay_generation.wrapping_add(1); + self.decay_all_counters(); + } + } } /// counter.py:309 DeterministicJitCounter — test-only, NOT_RPYTHON. @@ -371,6 +396,25 @@ impl DeterministicJitCounter { #[cfg(test)] mod tests { use super::*; + use std::sync::Mutex; + + static DECAY_GENERATION_TEST_LOCK: Mutex<()> = Mutex::new(()); + + fn advance_decay_generation(intervals: usize) { + DECAY_GENERATION.fetch_add(intervals, Ordering::Relaxed); + } + + fn counter_time(counter: &JitCounter, hash: u64) -> f32 { + let index = counter._get_index(hash); + let subhash = JitCounter::_get_subhash(hash); + let entry = &counter.timetable[index]; + for i in 0..ASSOCIATIVITY { + if entry.subhashes[i] == subhash { + return entry.times[i]; + } + } + 0.0 + } #[test] fn test_basic_counting() { @@ -434,6 +478,46 @@ mod tests { assert!(time > 0.7 && time < 0.8, "time={}", time); } + #[test] + fn test_tick_applies_every_pending_decay_generation() { + let _generation_guard = DECAY_GENERATION_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let mut counter = JitCounter::new(DEFAULT_SIZE); + counter.set_decay(40); + let h = 3u64 << counter.shift; + counter.change_current_fraction(h, 0.5); + + advance_decay_generation(2); + let increment = 0.001; + assert!(!counter.tick(h, increment)); + + let expected = + (((0.5f32 as f64 * 0.96) as f32 as f64 * 0.96) as f32 as f64 + increment) as f32; + let actual = counter_time(&counter, h); + assert!((actual - expected).abs() < 1.0e-6, "actual={actual}"); + } + + #[test] + fn test_change_current_fraction_is_not_retro_decayed() { + let _generation_guard = DECAY_GENERATION_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let mut counter = JitCounter::new(DEFAULT_SIZE); + counter.set_decay(40); + let h = 4u64 << counter.shift; + counter.change_current_fraction(h, 0.5); + + advance_decay_generation(1); + counter.change_current_fraction(h, 0.98); + let increment = 0.001; + assert!(!counter.tick(h, increment)); + + let expected = (0.98f32 as f64 + increment) as f32; + let actual = counter_time(&counter, h); + assert!((actual - expected).abs() < 1.0e-6, "actual={actual}"); + } + #[test] fn test_auto_reset_on_fire() { let mut counter = JitCounter::new(DEFAULT_SIZE); From 521efc3950c48885a7029f5fe318946031e2da5e Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Fri, 14 Aug 2026 03:19:55 +0900 Subject: [PATCH 2/2] jit: re-record the three baselines the counter decay moved, and correct two stale notes The decay drain lands one more decay interval per counter than before, so two synthetic fixtures settle on different counters: inline_chain_depth_typeflip dynasm+cranelift bridges_compiled 19 -> 18, guard_failures 3820 -> 3681 exception_traceback_loop_forms wasm guard_failures 810 -> 811 Re-recorded from a full local gate at 4a391053e79 (dynasm 423/425, cranelift 424/425, wasm 417/418; the remaining dynasm failure is the intermittent minor_remembered_set GC panic, which fires on a different bench each full run). check.py's baseline-resolution note claimed a fresh local build reports the ubuntu str_fstring pair. It reports the macos-latest pair: two full local gates pass against the plain `.darwin` overlays holding dynasm 659 and cranelift 658. The darwin GitHub-runner overlay for cranelift held the same value as the platform-wide one, so it could never be reached and is removed. counter.rs described the deferred decay as avoiding a borrow the GC does not hold. The accessor for the JIT_DRIVER cell mints a `&'static mut JitDriverPair`, and a minor collection can be triggered by an allocation the metainterp makes while already holding one, so decaying from inside the collector would alias it. Assisted-by: Claude --- majit/majit-trace/src/counter.rs | 11 ++++++---- ...ception_traceback_loop_forms.wasm.jitstats | 3 ++- ...ne_chain_depth_typeflip.cranelift.jitstats | 5 +++-- ...nline_chain_depth_typeflip.dynasm.jitstats | 5 +++-- ...g.cranelift.darwin.github-actions.jitstats | 15 ------------- pyre/check.py | 21 +++++++++++-------- 6 files changed, 27 insertions(+), 33 deletions(-) delete mode 100644 pyre/bench/synth/str_fstring.cranelift.darwin.github-actions.jitstats diff --git a/majit/majit-trace/src/counter.rs b/majit/majit-trace/src/counter.rs index 6ed277bbc73..c62def5e6dd 100644 --- a/majit/majit-trace/src/counter.rs +++ b/majit/majit-trace/src/counter.rs @@ -182,12 +182,15 @@ impl JitCounter { #[inline(always)] pub fn tick(&mut self, hash: u64, increment: f64) -> bool { // counter.py:104-121 applies the decay synchronously inside the minor - // collection. pyre defers it to the next table access because the - // metainterp owns the counter table, and reaching it from inside the - // collector would re-enter a borrow the GC does not hold. + // collection, where the hook closes over the process's one JitCounter. + // pyre defers it to the next table access instead: the counter is + // reached through the `JIT_DRIVER` cell in eval.rs, whose accessor + // mints a `&'static mut JitDriverPair`, and a minor collection can be + // triggered by an allocation the metainterp makes while already holding + // one. Decaying from inside the collector would alias it. // // Each JitCounter keeps its own last-seen generation because JIT_DRIVER - // in eval.rs is thread-local, giving each mutator thread its own counter. + // is thread-local, giving each mutator thread its own counter. // Every elapsed interval is applied before every mutating table access; // would_tick_fire decay-adjusts its read. A value written after a // collection is therefore not retro-decayed. diff --git a/pyre/bench/synth/exception_traceback_loop_forms.wasm.jitstats b/pyre/bench/synth/exception_traceback_loop_forms.wasm.jitstats index 662db91dac1..a2969afa472 100644 --- a/pyre/bench/synth/exception_traceback_loop_forms.wasm.jitstats +++ b/pyre/bench/synth/exception_traceback_loop_forms.wasm.jitstats @@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=810 +guard_failures=811 internal_compile_panics=0 loops_aborted=0 loops_compiled=8 +retraces_compiled=0 diff --git a/pyre/bench/synth/inline_chain_depth_typeflip.cranelift.jitstats b/pyre/bench/synth/inline_chain_depth_typeflip.cranelift.jitstats index 3a3b8ddb6b6..851c5b8bdf9 100644 --- a/pyre/bench/synth/inline_chain_depth_typeflip.cranelift.jitstats +++ b/pyre/bench/synth/inline_chain_depth_typeflip.cranelift.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=19 +bridges_compiled=18 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 @@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=3820 +guard_failures=3681 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 +retraces_compiled=0 diff --git a/pyre/bench/synth/inline_chain_depth_typeflip.dynasm.jitstats b/pyre/bench/synth/inline_chain_depth_typeflip.dynasm.jitstats index 3a3b8ddb6b6..851c5b8bdf9 100644 --- a/pyre/bench/synth/inline_chain_depth_typeflip.dynasm.jitstats +++ b/pyre/bench/synth/inline_chain_depth_typeflip.dynasm.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=19 +bridges_compiled=18 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 @@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=3820 +guard_failures=3681 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 +retraces_compiled=0 diff --git a/pyre/bench/synth/str_fstring.cranelift.darwin.github-actions.jitstats b/pyre/bench/synth/str_fstring.cranelift.darwin.github-actions.jitstats deleted file mode 100644 index 5c093f4f38b..00000000000 --- a/pyre/bench/synth/str_fstring.cranelift.darwin.github-actions.jitstats +++ /dev/null @@ -1,15 +0,0 @@ -bridges_compiled=3 -descr_set_absent=0 -descr_set_ambiguous=0 -descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=0 -fbw_rolled_back_with_effects=0 -fbw_store_journal_rollback_failed=0 -field_pos_attached_misplaced=0 -field_pos_spec_misplaced=0 -guard_failures=658 -internal_compile_panics=0 -loops_aborted=0 -loops_compiled=6 -retraces_compiled=0 diff --git a/pyre/check.py b/pyre/check.py index 05e964be0c0..b0bd461342d 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -1537,11 +1537,17 @@ def _jitstats_baseline_path(self, backend, script): # what let eight baselines go stale unnoticed. # # Measured, not assumed, and read back from the runner jobs rather than - # predicted. A fresh local build and ubuntu-24.04 report str_fstring - # guard_failures 658/659 on dynasm/cranelift. macos-latest reports the - # inverse 659/658, and windows-latest reports 658/658. Every other gated - # counter is identical (six loops and three bridges), and each mismatch - # reproduced in both stability reruns. + # predicted. ubuntu-24.04 reports str_fstring guard_failures 658/659 on + # dynasm/cranelift. macos-latest reports the inverse 659/658, and + # windows-latest reports 658/658. Every other gated counter is identical + # (six loops and three bridges), and each mismatch reproduced in both + # stability reruns. + # + # A local arm64 macOS build observes the macos-latest pair rather than + # ubuntu's: two full local gates pass against plain `.darwin` overlays + # holding dynasm 659 and cranelift 658. Darwin is therefore carried + # platform-wide, and the darwin GitHub-runner overlay that once held the + # same cranelift value was dropped as unreachable. # # This is a collection-schedule boundary, not a compile-shape change. # On a local arm64 macOS cranelift build, adding check.py's two wasm @@ -1552,10 +1558,7 @@ def _jitstats_baseline_path(self, backend, script): # is where a collection-triggered breaker lands, not how many loops or # bridges compile. The runner did not enable either census, so exactly # which host input moves that placement remains unidentified; the - # observed counter and stable runner split are direct measurements. The - # fresh local macOS build matches ubuntu rather than the macOS runner, - # which is why these are GitHub-runner overlays rather than - # platform-wide ones. + # observed counter and stable runner split are direct measurements. # # An overlay shadows the shared baseline permanently, so one written # against a value that later converges becomes a failure main does not