Replace per-run record maps with a slot-indexed arena table - #11107
Merged
kalenikaliaksandr merged 2 commits intoAug 12, 2026
Merged
Conversation
subgrid_item_contributions_to_track_sizing already binds the subgrid's live record before building its scratch measurement run, but re-fetched it from the parent run's records afterwards. Use the captured handle for the two remaining reads so nothing consults the parent table while the scratch run shadows the subgrid's slot.
When used-values records moved from the pass-global PagedStore into run-scoped ownership, lookups regressed from an indexed page-table load to a per-run HashMap: a RefCell borrow, a SipHash of the slot index, a bucket probe, and an Rc clone, adding up to roughly a fifth of WebContent's layout profile on table- and flex-heavy documents. Restore indexed lookups without giving up run scoping by storing the records in one slot-indexed side table in the layout node arena, sized with the slot space and stamped with a per-run nonce: a read becomes an indexed load and a compare. Runs strictly nest on the call stack, so a run that registers a slot another live run owns displaces that entry into its undo list and restores it on drop, which keeps the ownership semantics (and the unowned-read panics) exactly as before. Record reads drop from ~57ns to ~9ns and registrations halve, which translates to 23-31% faster layout passes on documents dominated by large tables, deep block nesting, or many flex/grid items.
📝 WalkthroughWalkthrough
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs`:
- Around line 855-870: Replace the debug-only lifecycle checks with
release-enforced assertions: in restore_run_record, use assert_eq! to reject
non-LIFO restoration before overwriting the slot, and at the run-record slot
freeing logic around lines 350-358, use assert! or assert_eq! to reject clearing
a slot that still contains a live record. Apply both changes in
Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs at the specified ranges.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5af11a93-e19c-4067-add3-a9c95ab0ce3a
📒 Files selected for processing (5)
Libraries/LibWeb/Rust/src/layout/formatting_context.rsLibraries/LibWeb/Rust/src/layout/grid_formatting_context.rsLibraries/LibWeb/Rust/src/layout/layout_node_arena.rsLibraries/LibWeb/Rust/src/layout/run_records.rsLibraries/LibWeb/Rust/src/layout/sizing_context.rs
Comment on lines
+855
to
+870
| pub(crate) fn restore_run_record(&self, slot_index: u32, run_nonce: u64, previous: Option<(u64, Rc<UsedValues>)>) { | ||
| let mut records = self.run_used_records.borrow_mut(); | ||
| let slot = records | ||
| .get_mut(slot_index as usize) | ||
| .expect("restored layout run record slot must exist"); | ||
| debug_assert_eq!( | ||
| slot.nonce, run_nonce, | ||
| "layout run records were not restored in LIFO order" | ||
| ); | ||
| *slot = match previous { | ||
| Some((nonce, record)) => RunRecordSlot { | ||
| nonce, | ||
| record: Some(record), | ||
| }, | ||
| None => RunRecordSlot::default(), | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep run-record lifecycle checks enabled in release builds.
debug_assert! and debug_assert_eq! are removed in release builds. A non-LIFO restore can overwrite another live run's record. Freeing a slot with a live record can clear that record before the run drops. Use assert! and assert_eq! at both sites.
Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs#L855-L870: reject a non-LIFO restoration before writing the slot.Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs#L350-L358: reject freeing a slot that still has a live run record before clearing it.
📍 Affects 1 file
Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs#L855-L870(this comment)Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs#L350-L358
🤖 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 `@Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs` around lines 855 -
870, Replace the debug-only lifecycle checks with release-enforced assertions:
in restore_run_record, use assert_eq! to reject non-LIFO restoration before
overwriting the slot, and at the run-record slot freeing logic around lines
350-358, use assert! or assert_eq! to reject clearing a slot that still contains
a live record. Apply both changes in
Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs at the specified ranges.
kalenikaliaksandr
enabled auto-merge (rebase)
August 12, 2026 11:00
kalenikaliaksandr
disabled auto-merge
August 12, 2026 11:12
kalenikaliaksandr
merged commit Aug 12, 2026
0f594af
into
LadybirdBrowser:master
14 of 15 checks passed
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.
When used-values records moved from the pass-global PagedStore into
run-scoped ownership, lookups regressed from an indexed page-table load
to a per-run HashMap: a RefCell borrow, a SipHash of the slot index, a
bucket probe, and an Rc clone, adding up to roughly a fifth of
WebContent's layout profile on table- and flex-heavy documents. Restore
indexed lookups without giving up run scoping by storing the records in
one slot-indexed side table in the layout node arena, sized with the
slot space and stamped with a per-run nonce: a read becomes an indexed
load and a compare. Runs strictly nest on the call stack, so a run that
registers a slot another live run owns displaces that entry into its
undo list and restores it on drop, which keeps the ownership semantics
(and the unowned-read panics) exactly as before.
Record reads drop from ~57ns to ~9ns and registrations halve, which
translates to 23-31% faster layout passes on documents dominated by
large tables, deep block nesting, or many flex/grid items.