Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
bridges_compiled=6
bridges_compiled=4
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
guard_failures=1203
guard_failures=802
internal_compile_panics=0
loops_aborted=1
loops_aborted=0
loops_compiled=2
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
guard_failures=8
guard_failures=5
internal_compile_panics=0
loops_aborted=1
loops_aborted=0
loops_compiled=8
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
bridges_compiled=2
bridges_compiled=4
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
bridges_compiled=0
bridges_compiled=1
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
bridges_compiled=3
bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
bridges_compiled=10
bridges_compiled=7
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
guard_failures=2018
guard_failures=1655
internal_compile_panics=0
loops_aborted=1
loops_aborted=0
loops_compiled=4
4 changes: 2 additions & 2 deletions pyre/bench/synth/list_length_hint_validate.dynasm.jitstats
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
bridges_compiled=4
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
guard_failures=828
guard_failures=1
internal_compile_panics=0
loops_aborted=10
loops_compiled=2
4 changes: 2 additions & 2 deletions pyre/bench/synth/slots_class_var_conflict.dynasm.jitstats
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ descr_set_ambiguous=0
descr_set_stale_absent=0
guard_failures=1
internal_compile_panics=0
loops_aborted=5
loops_compiled=3
loops_aborted=0
loops_compiled=5
9 changes: 6 additions & 3 deletions pyre/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -676,9 +676,12 @@ def _jit_stats_change(saved, current):

A field missing from either side reads as "0", so a baseline recorded before
a counter existed still matches a run that reports it as 0, and adding an
invariant counter costs no re-record. The wasm [jit-stats] line reports only
the badness fields, so its baselines carry only those and stay equal on the
three it never prints."""
invariant counter costs no re-record. The cost of that convenience is that a
field absent from BOTH sides compares equal forever: a backend whose
[jit-stats] line stops naming a counter disarms that counter's gate on every
one of its baselines, silently. Whenever a backend's line changes shape,
re-record its whole baseline surface rather than trusting the run that
follows."""
Comment on lines +679 to +684

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Qualify the missing-counter warning.

When current omits a field but saved contains a non-zero value, _jit_stats_change compares that value with "0" and reports a change. The gate is silently disabled only when both snapshots omit the field, or when the saved value is already zero.

Replace “disarms that counter’s gate on every one of its baselines” with wording that reflects this behavior.

Suggested wording
-    field absent from BOTH sides compares equal forever: a backend whose
-    [jit-stats] line stops naming a counter disarms that counter's gate on every
-    one of its baselines, silently.
+    field absent from BOTH sides compares equal forever. If a backend stops
+    naming a counter, only baselines with a saved value of zero are silently
+    disabled; a non-zero saved value still compares against zero and fails.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
invariant counter costs no re-record. The cost of that convenience is that a
field absent from BOTH sides compares equal forever: a backend whose
[jit-stats] line stops naming a counter disarms that counter's gate on every
one of its baselines, silently. Whenever a backend's line changes shape,
re-record its whole baseline surface rather than trusting the run that
follows."""
invariant counter costs no re-record. The cost of that convenience is that a
field absent from BOTH sides compares equal forever. If a backend stops
naming a counter, only baselines with a saved value of zero are silently
disabled; a non-zero saved value still compares against zero and fails.
Whenever a backend's line changes shape, re-record its whole baseline surface
rather than trusting the run that follows."""
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/check.py` around lines 679 - 684, Update the explanatory comment near
_jit_stats_change to qualify that a missing field in current disables the gate
only when the corresponding field is also absent from saved or saved already has
a zero value; retain the existing behavior for non-zero saved values, which are
reported as changes.

Comment on lines +682 to +684

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not re-record after an emitted counter disappears

When a backend stops emitting a counter whose saved value is nonzero, _jit_stats_change does not silently disarm the gate: the missing current value defaults to 0, so the comparison fails. Following this new advice with --snapshot is what actually disarms it, because record mode bypasses the comparison and rewrites the baseline without the field; subsequent runs then compare missing against missing. A disappearing counter should therefore be fixed or explicitly removed from the gated surface, not handled by re-recording.

Useful? React with 👍 / 👎.

old_fields = _parse_jit_stats(saved)
new_fields = _parse_jit_stats(current)
changes = [
Expand Down
Loading