fix(hir): recognize the Bun platform global in diagnostics - #9754
Closed
proggeramlug wants to merge 2 commits into
Closed
fix(hir): recognize the Bun platform global in diagnostics#9754proggeramlug wants to merge 2 commits into
proggeramlug wants to merge 2 commits into
Conversation
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used all 8 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
Landed on |
proggeramlug
pushed a commit
to proggeramlug/perry
that referenced
this pull request
Sep 5, 2026
… % more The five tables PerryTS#9754 converted were valued individually with a measurement-only `PERRY_YOUNG_LOG=0` gate on `RuntimeRootVisitor::young_scope()` (all five scanners fall back to their full walk together inside one binary), plus a third arm — `cc_base_new`, main `1d63fa91f`, no logs at all. Three interleaved rounds, `stream_scale` len 3300, identical collection schedule in every arm (minors 196/194/196, budgeted steps 60/59/59), so these are scan costs: | scanner, ms per turn | main | log, full walk | log, minor walk | |------------------------------|-----------------|-----------------|-----------------| | all 95 scanners | 14761/14105/19411 | 23368/23286/26974 | 2667/2852/3674 | | scan_shape_table_rekey_mut | 10884/11077/14458 | 18893/18655/22125 | 1426/1567/1947 | | scan_descriptor_roots_mut | 1807/1192/2223 | 2257/2467/2548 | 127/136/145 | | scan_closure_dynamic_props | 1014/985/1347 | 890/902/959 | 224/236/209 | | transition_cache scanner | 121/123/159 | 254/221/246 | 85/94/145 | | shape_cache scanner | 89/89/113 | 159/153/167 | 121/122/151 | The shape cache is the one table where the log loses to the walk it replaced: +34 ms (+35 %) against main, having skipped **0.0 % of 3.85 M entry visits in every one of 107 collections**. The cause was already documented — the canonical keys arrays are allocated in the LONGLIVED arena, which `addr_is_minor_relevant` must answer `true` for because a longlived parent is not write-barriered, and a longlived object is never promoted, so no entry ever leaves the log. So it goes back to the plain `values_mut()` walk: the arm helper, its production and test-seam call sites, the thread-local log, the name constant and the `debug_assert_logged` re-derivation are all deleted. An inert log is not free — it is a permanent arming obligation on every future writer of that cache plus a suppression audit that has to keep proving each site — and it should not land on the promise of a longlived remembered set that does not exist yet. When that set exists and makes this table skip something, the log can come back with a measurement. The test is kept as a scanner test (a young entry reachable only through the cache still moves and is re-keyed in both the inline slot and the overflow map) and now asserts that NO `[gc-young-log]` row exists for the table, so re-adding a log here without re-measuring is a red test. Note for anyone repeating this on another table: the two-arm version of this experiment gives the wrong answer. With the log merely disabled, the full-walk arm still pays its upkeep — a `take_sorted()` whose sorted result is discarded and an `addr_is_minor_relevant` probe per entry to rebuild `kept` — so every "off" row above is worse than main, by +7.8 s on the shapes table alone. Only the third arm says whether a log should exist at all. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m
proggeramlug
pushed a commit
that referenced
this pull request
Sep 5, 2026
proggeramlug
pushed a commit
to proggeramlug/perry
that referenced
this pull request
Sep 5, 2026
… % more The five tables PerryTS#9754 converted were valued individually with a measurement-only `PERRY_YOUNG_LOG=0` gate on `RuntimeRootVisitor::young_scope()` (all five scanners fall back to their full walk together inside one binary), plus a third arm — `cc_base_new`, main `1d63fa91f`, no logs at all. Three interleaved rounds, `stream_scale` len 3300, identical collection schedule in every arm (minors 196/194/196, budgeted steps 60/59/59), so these are scan costs: | scanner, ms per turn | main | log, full walk | log, minor walk | |------------------------------|-----------------|-----------------|-----------------| | all 95 scanners | 14761/14105/19411 | 23368/23286/26974 | 2667/2852/3674 | | scan_shape_table_rekey_mut | 10884/11077/14458 | 18893/18655/22125 | 1426/1567/1947 | | scan_descriptor_roots_mut | 1807/1192/2223 | 2257/2467/2548 | 127/136/145 | | scan_closure_dynamic_props | 1014/985/1347 | 890/902/959 | 224/236/209 | | transition_cache scanner | 121/123/159 | 254/221/246 | 85/94/145 | | shape_cache scanner | 89/89/113 | 159/153/167 | 121/122/151 | The shape cache is the one table where the log loses to the walk it replaced: +34 ms (+35 %) against main, having skipped **0.0 % of 3.85 M entry visits in every one of 107 collections**. The cause was already documented — the canonical keys arrays are allocated in the LONGLIVED arena, which `addr_is_minor_relevant` must answer `true` for because a longlived parent is not write-barriered, and a longlived object is never promoted, so no entry ever leaves the log. So it goes back to the plain `values_mut()` walk: the arm helper, its production and test-seam call sites, the thread-local log, the name constant and the `debug_assert_logged` re-derivation are all deleted. An inert log is not free — it is a permanent arming obligation on every future writer of that cache plus a suppression audit that has to keep proving each site — and it should not land on the promise of a longlived remembered set that does not exist yet. When that set exists and makes this table skip something, the log can come back with a measurement. The test is kept as a scanner test (a young entry reachable only through the cache still moves and is re-keyed in both the inline slot and the overflow map) and now asserts that NO `[gc-young-log]` row exists for the table, so re-adding a log here without re-measuring is a red test. Note for anyone repeating this on another table: the two-arm version of this experiment gives the wrong answer. With the log merely disabled, the full-walk arm still pays its upkeep — a `take_sorted()` whose sorted result is discarded and an `addr_is_minor_relevant` probe per entry to rebuild `kept` — so every "off" row above is worse than main, by +7.8 s on the shapes table alone. Only the third arm says whether a log should exist at all. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In
--platform bunmode, ordinary reads ofBunemitted unknown-identifier warnings even though the compiler installsglobalThis.Bunbefore module initialization. Pass the selected platform's supplied global names into HIR lowering and suppress that warning for an unshadowed supplied name, including the constructor-reference diagnostic.Reads keep the existing by-name runtime lookup, so replacing
globalThis.Bunremains observable. Lexical bindings still resolve first. Default-platform diagnostics and unrelated unknown names keep their existing behavior. The existing public lowering entry point retains its signature and uses an empty supplied-global set.Validation:
PERRY_CONCAT_SITE_CACHEregistration gap, fixed separately by fix(cache): register concat switch and explain codegen inputs #9748.Other baseline CI failures are addressed separately: raw TLS policy by #9750, and Linux pthread declaration warnings by #9752.
Closes #9745. No version bump.