perf(runtime): typed-feedback store forwards when recording is off; one-entry cache for the external-slot remembered set (ECS round 4) - #8951
Conversation
…ecording is off; a one-entry cache in front of the external-slot remembered set js_typed_feedback_array_set_index_or_string converted the index and made two out-of-line calls whose first statement is "recording is off, return" — 1.5% of an ECS frame on `column[index] = record`. One flag test, then the strict store. A Map's entries buffer is an external slot span, so every map.set(k, v) on an old map reached mark_dirty_external_slot_page: a thread-local table probe plus a linear scan of the page's header list, which grows with every map whose buffer shares the page. The (page, header) pair is now cached inline in HotTls — recorded only after the table holds it, dropped wherever the table drops a pair — so a repeated store into the same map answers from two loads, exactly as the inline-slot dirty-page cache does. Claude-Session: https://claude.ai/code/session_01FUvFrRNZyc5qknBiJbYbby
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe runtime adds a direct strict-store path when typed feedback is disabled. It also adds a one-entry hot-TLS cache for repeated external dirty-slot pairs and invalidates the cache when entries are cleared. ChangesRuntime optimizations
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR makes localized runtime performance changes while preserving array-store behavior and remembered-set cache invalidation. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (1 skipped: 1 unsupported.) Full details: Description checkExplanation The description explains the two changes and reports test coverage, but it does not use the required section headings or provide the required related-issue, test-plan checklist, and contributor checklist information. Resolution Reformat the description using the repository template. Add Summary, Changes, Related issue with an issue reference or "n/a", Test plan with verification commands and completed checkboxes, Screenshots / output if applicable, and the required Checklist entries.
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Gate on the isolated perrymaster clone for |
|
Paired measurement (idle Mac mini, 9 alternating pairs, |
1 participant
Two runtime mechanisms from the ECS round-4 chain, cut from current main. Suites on the isolated perrymaster gate: runtime (2752, incl. the new external-cache test). Paired measurement on the
codehz/ecs"5k entities: 3 commands each + sync" row (idle Mac mini, alternating pairs) follows in a comment.js_typed_feedback_array_set_index_or_stringconverted the index and made two out-of-line calls (observe_array,record_guard_pass) whose first statement is "recording is off, return" — 1.5% of the merged-main ECS frame oncolumn[index] = record. One flag test (typed_feedback_enabled, the default is off), then the strict store; the recording path is unchanged when the env enables it.Map's entries buffer is an external slot span, so everymap.set(k, v)on an old map reachedmark_dirty_external_slot_page: a thread-local table probe plus a linear scan of that page's header list (which grows with every map whose buffer shares the page) — the two hottest offsets ofjs_map_seton merged main. The(page, header)pair is cached inline inHotTls, recorded only after the table holds it and dropped wherever the table drops a pair (clear_one_external_dirty_slot_header, which every clear path goes through), so a repeated store into the same map answers from two loads — the external-slot twin of the inline-slot dirty-page cache (perf: ECS round 4 — barrier early exits, inline captureless some loop, lean Map/Set lanes, empty-pop fast path, codegen-time const fold, inline hot-TLS values (−16.7%) #8916).external_dirty_slot_pair_cache_mirrors_the_tablepins hit / miss / re-record-after-clear.https://claude.ai/code/session_01FUvFrRNZyc5qknBiJbYbby
Summary by CodeRabbit
Performance
Reliability
Tests
Documentation