-
Notifications
You must be signed in to change notification settings - Fork 19
jit(wasm): drop the wasm32 arm of the self-recursive root-bridge inline #1106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
a62f004
95ec7f2
203cd0c
87c0439
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,14 @@ | ||
| bridges_compiled=26 | ||
| bridges_compiled=27 | ||
| 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=5165 | ||
| guard_failures=5150 | ||
| internal_compile_panics=0 | ||
| loops_aborted=2 | ||
| loops_aborted=1 | ||
| loops_compiled=3 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| bridges_compiled=6 | ||
| 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=806 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=2 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| bridges_compiled=6 | ||
| 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=806 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=2 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # pyre-check: max-pypy-ratio=20 | ||
| # Coverage for the self-recursive root-bridge inline when the recursion is | ||
| # non-tail and carries a Ref local. | ||
| # | ||
| # `walk` is self-recursive, takes exact-integer arguments and holds a | ||
| # `BinaryOp` residual, so a guard-failure bridge that reaches its CALL takes the | ||
| # root-bridge admission (`bridge_rec_root_selfrec`, inline_call.rs). | ||
| # `bridge_recursion_overflow` already covers that admission, but only in its | ||
| # easiest form: tail recursion whose live set is two machine integers. Two | ||
| # ingredients of the "a Ref reached an int operation" failure it is meant to | ||
| # guard against were therefore unexercised. | ||
| # | ||
| # `acc * 2` crosses the machine-int boundary partway down the recursion, so the | ||
| # accumulator promotes to a long — a Ref — at a level that moves with the | ||
| # caller's seed, and the overflow guard fires inside the recursive frame. `tag` | ||
| # keeps a second Ref live across the recursive CALL beside it, and the non-tail | ||
| # `inner + len(tag)` leaves a paused caller chain, so the guard's resume stream | ||
| # is multi-frame and mixes Ref with Int rather than being one frame of | ||
| # integers. | ||
| # | ||
| # Byte-parity against CPython/PyPy is the gate: Python integers are unbounded, | ||
| # so the promotion must not be observable in the result. | ||
| _TAGS = ("a", "bb", "ccc", "dddd") | ||
|
|
||
|
|
||
| def walk(n, acc): | ||
| if n == 0: | ||
| return acc | ||
| tag = _TAGS[n & 3] | ||
| nxt = acc * 2 + len(tag) | ||
| inner = walk(n - 1, nxt) | ||
| return inner + len(tag) | ||
|
|
||
|
|
||
| out = [] | ||
| for i in range(50000): | ||
| out.append(walk(24, (1 << 50) + i) % 1000000007) | ||
| print(out[0], out[-1], sum(out) % 1000000007) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| bridges_compiled=6 | ||
| 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=806 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=2 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,14 @@ | ||
| bridges_compiled=1 | ||
| 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 | ||
| guard_failures=404 | ||
| fbw_store_journal_rollback_failed=0 | ||
| field_pos_attached_misplaced=0 | ||
| field_pos_spec_misplaced=0 | ||
| guard_failures=601 | ||
| internal_compile_panics=0 | ||
| loops_aborted=1 | ||
| loops_compiled=2 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2865,13 +2865,10 @@ pub(crate) fn try_walker_inline_resolved_user_call<Sym: WalkSym>( | |
| // body sub-walk reaches its own recursive CALL as a nested residual, which | ||
| // `fbw_abort_nested_unjournaled_residual` declines on the self-recursive | ||
| // hazard arm — an abort storm that folds the whole guard bridge back to | ||
| // residual. The native `CALL_ASSEMBLER` self-recursion fold already exempts | ||
| // that decline via `SELFREC_CA_FOLD_ACTIVE`; the same exemption applies to | ||
| // this admitted inline, whose recursive residual runs concretely at the | ||
| // pre-execute site (executed, so no replay double-apply). Native only: the | ||
| // wasm always-portal path type-confuses the self-recursive inline | ||
| // (`setintbound: got Ref`), so it keeps the correct residual-fallback | ||
| // decline. | ||
| // residual. The `CALL_ASSEMBLER` self-recursion fold already exempts that | ||
| // decline via `SELFREC_CA_FOLD_ACTIVE`; the same exemption applies to this | ||
| // admitted inline, whose recursive residual runs concretely at the | ||
| // pre-execute site (executed, so no replay double-apply). | ||
| let mut bridge_rec_root_selfrec = false; | ||
| if ctx.trace_ctx.is_bridge_trace | ||
| && args_all_builtin_integer | ||
|
|
@@ -2911,12 +2908,11 @@ pub(crate) fn try_walker_inline_resolved_user_call<Sym: WalkSym>( | |
| if !safe_root_bridge { | ||
| return Ok(None); | ||
| } | ||
| bridge_rec_root_selfrec = cfg!(not(target_arch = "wasm32")) | ||
| && unsafe { | ||
| let raw = pyre_interpreter::w_code_get_ptr(w_code as pyre_object::PyObjectRef) | ||
| as *const pyre_interpreter::CodeObject; | ||
| !raw.is_null() && pyre_interpreter::code_is_self_recursive(&*raw) | ||
| }; | ||
| bridge_rec_root_selfrec = unsafe { | ||
| let raw = pyre_interpreter::w_code_get_ptr(w_code as pyre_object::PyObjectRef) | ||
| as *const pyre_interpreter::CodeObject; | ||
| !raw.is_null() && pyre_interpreter::code_is_self_recursive(&*raw) | ||
| }; | ||
|
Comment on lines
+2918
to
+2922
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This removal newly enables the exemption on wasm whenever AGENTS.md reference: AGENTS.md:L14-L19 Useful? React with 👍 / 👎. |
||
| } | ||
| // A callee `fbw_abort_nested_unjournaled_residual` already named on its | ||
| // hazard arm residualizes from here on. The hazard is a static property of | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: youknowone/pyre
Length of output: 21136
🌐 Web query:
pyproject.toml ruff PERF401 preferred method build list comprehension💡 Result:
The Ruff rule PERF401 (manual-list-comprehension) identifies for-loops that can be replaced with a list comprehension or a list extend operation to improve readability and performance [1][2]. The preferred method depends on the context of the code: 1. When initializing a new list: Ruff suggests replacing the loop with a list comprehension [1][3]. Example: # Original filtered = [] for i in original: if i % 2: filtered.append(i) # Preferred (List Comprehension) filtered = [i for i in original if i % 2] 2. When appending to an existing, already populated list: Ruff suggests using the list.extend method with a generator expression [1][4]. Example: # Original existing_list = [1][5] for i in original: if i % 2: existing_list.append(i) # Preferred (list.extend) existing_list.extend(i for i in original if i % 2) Key Considerations: - Purpose: The rule is a micro-optimization [1][2]. While list comprehensions are generally faster (typically 10-25% depending on the Python version), the performance impact is negligible in many real-world scenarios [1][2]. - Limitations: The automatic fix is not always applicable. Ruff applies this rule primarily to simple for-loops (e.g., those with a single non-if statement or a single if statement containing one nested operation) [6][4]. If a loop contains complex logic or multiple operations, it may not trigger the rule [6]. - False Positives: If the logic requires flattening (which.extend does) versus adding elements, or if the loop body is more complex than the rule's simple heuristic can analyze, you should manually evaluate if the suggested transformation maintains correct behavior [4].
Citations:
perflint] Allow list function calls to be replaced with a comprehension (PERF401) astral-sh/ruff#17519perflint] AddPERF401andPERF402rules astral-sh/ruff#5298Resolve Ruff
PERF401without changing the benchmark shape.This initialization loop triggers
PERF401, but converting it to alist(...)construction may change the JIT trace and invalidate the.jitstatsbaselines. Add a local suppression if the explicit loop is intentional, or regenerate all threeselfrec_bridge_nontail_promoteJIT-stat files if the comprehension is kept.Proposed localized suppression
📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 37-37: Use a list comprehension to create a transformed list
(PERF401)
🤖 Prompt for AI Agents
Source: Linters/SAST tools