refactor(core): reduce layout/render hot-path allocations#228
refactor(core): reduce layout/render hot-path allocations#228RtlZeroMemory merged 8 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces string-based layout cache with a nested map hierarchy, enlarges the array pool, rewrites stack layout into a two-phase sizing + cross-axis feedback flow with pooling, makes render-packet recorder internals mutable for reuse and optional-field emission tighter, and guards theme-override merging in render trees. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/layout/engine/layoutEngine.ts (1)
89-89: Add a fail-fast guard to prevent sentinel/key collisions.If a negative forced dimension ever regresses in upstream callers,
nulland-1currently collide in cache keying. A lightweight guard here prevents silent wrong-cache hits.Suggested patch
const NULL_FORCED_DIMENSION = -1; function forcedDimensionKey(value: number | null): number { - return value === null ? NULL_FORCED_DIMENSION : value; + if (value === null) return NULL_FORCED_DIMENSION; + if (value < 0) { + throw new Error("layout: forced dimensions must be >= 0"); + } + return value; }Also applies to: 113-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/layout/engine/layoutEngine.ts` at line 89, Add a fail-fast guard around any code paths that accept or key on a forcedDimension to prevent negative values colliding with the sentinel NULL_FORCED_DIMENSION (constant NULL_FORCED_DIMENSION = -1); specifically, at the start of the functions that build cache keys or take a forcedDimension parameter, assert/throw if forcedDimension < 0 and forcedDimension !== NULL_FORCED_DIMENSION (e.g., throw new RangeError("invalid forcedDimension")); apply the same check in the other places that use NULL_FORCED_DIMENSION (the other cache-keying/site where forcedDimension is read—the region noted as also applying to 113-115) so any regression to negative values fails fast instead of producing wrong cache hits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/layout/engine/layoutEngine.ts`:
- Line 89: Add a fail-fast guard around any code paths that accept or key on a
forcedDimension to prevent negative values colliding with the sentinel
NULL_FORCED_DIMENSION (constant NULL_FORCED_DIMENSION = -1); specifically, at
the start of the functions that build cache keys or take a forcedDimension
parameter, assert/throw if forcedDimension < 0 and forcedDimension !==
NULL_FORCED_DIMENSION (e.g., throw new RangeError("invalid forcedDimension"));
apply the same check in the other places that use NULL_FORCED_DIMENSION (the
other cache-keying/site where forcedDimension is read—the region noted as also
applying to 113-115) so any regression to negative values fails fast instead of
producing wrong cache hits.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/core/src/layout/engine/layoutEngine.tspackages/core/src/layout/engine/pool.tspackages/core/src/layout/kinds/stack.tspackages/core/src/renderer/renderToDrawlist/renderPackets.tspackages/core/src/renderer/renderToDrawlist/renderTree.ts
|
Addressed the CodeRabbit nit: added a fail-fast guard in |
Summary
This PR reduces allocation churn in the core layout/render hot paths without changing rendering behavior.
Changes in scope
packages/core/src/layout/engine/layoutEngine.tspackages/core/src/layout/engine/pool.ts8to32.spliceto O(1) swap-pop.packages/core/src/layout/kinds/stack.tstry/finallyreleases to avoid pool leaks.packages/core/src/renderer/renderToDrawlist/renderPackets.tsRenderPacketRecorder.buildPacket()now transfers ownership ofops/resourcesinstead of copying with.slice().blobByResourceIdallocation when packet has no resources.packages/core/src/renderer/renderToDrawlist/renderTree.tsmergeThemeOverridecalls when nothemeoverride is provided.Validation
npx tsc -b packages/core packages/node packages/create-rezinode scripts/run-tests.mjsnpx biome check packages/core/src/layout/engine/layoutEngine.ts packages/core/src/layout/engine/pool.ts packages/core/src/layout/kinds/stack.ts packages/core/src/renderer/renderToDrawlist/renderPackets.ts packages/core/src/renderer/renderToDrawlist/renderTree.tsManual PTY verification (starship)
300x68) and frame audit enabled.1 2 3 4 5 6 t q.node scripts/frame-audit-report.mjs ... --latest-pid) shows:hash_mismatch_backend_vs_worker=0bridge,engineering,crew,comms,cargo,settings.Summary by CodeRabbit
Performance Improvements
Stability & Layout
Bug Fixes