Skip to content

perf(core): cut peak live memory 17.3% (taffy capacity, MeasureCtx dedupe) - #356

Open
quake wants to merge 1 commit into
pocket-stack:mainfrom
quake:bench-optimize/sep3
Open

perf(core): cut peak live memory 17.3% (taffy capacity, MeasureCtx dedupe)#356
quake wants to merge 1 commit into
pocket-stack:mainfrom
quake:bench-optimize/sep3

Conversation

@quake

@quake quake commented Sep 3, 2026

Copy link
Copy Markdown

Summary

Cuts pocketjs-core peak live memory 1,429,840 -> 1,182,457 bytes (-17.3%) on the new deterministic membench journey, with zero behavior change (drawlist checksum byte-identical, 127 tests pass, wasm32 no_std build clean).

  • Drop the unread MeasureCtx.text duplicate of every text run
  • Rebuild the taffy tree at subtree-sized capacity, freeing the old maps first (slotmap geometric growth left ~160 dead slots x ~1 KB)
  • Slim MeasureCtx to the shaped size; drop per-level children clones in collect_subtree/collect_run

engine/core/examples/membench.rs is the new counting-allocator benchmark (boot -> steady -> churn -> burst; prints PEAK_LIVE_BYTES).

Allocation-churn trade-off (measured)

Structural rebuilds now re-allocate the taffy maps instead of reusing historical capacity:

baseline head
TOTAL_ALLOC_BYTES 11,416,332 14,755,184
ALLOC_COUNT 109,081 101,314 (−7.1%)

The +3.3 MB of total allocation bytes is a handful of large blocks per structural rebuild (recycled by the PSP arena's O(1) size-class free lists); the allocation count goes down.

Benchmark integrity

NODES reports the true live node count via a harness-side hierarchy model mirroring Ui::destroy_node's subtree semantics (a call-counting counter drifted by +35 after churn). DRAWLIST_CHECKSUM is byte-identical across baseline and head.

Note on CI

Workflow runs on this fork PR require maintainer approval (action_required); the numbers above are local runs of cargo test, cargo run --example membench, and cargo build --target wasm32-unknown-unknown, reproducible with the commands in the diff.

@quake
quake force-pushed the bench-optimize/sep3 branch from ee2e5e0 to 4983fac Compare September 3, 2026 05:00
@quake
quake marked this pull request as ready for review September 3, 2026 05:03
@quake
quake force-pushed the bench-optimize/sep3 branch from 4983fac to 23e06a7 Compare September 3, 2026 14:00
@HalfSweet

Copy link
Copy Markdown
Collaborator

Thanks for the optimization work. Removing the duplicated data from MeasureCtx and avoiding children.clone() during recursive traversal both look reasonable.

My main concern is that the current benchmark measures live bytes using the host allocator, while the actual PSP constraint is the arena high-water mark. The benchmark also includes some allocations from the test harness itself, so the current results are not enough to show that rebuilding and resizing the Taffy tree on every structural relayout is a net improvement on PSP.

There are also a few unrelated or incomplete benchmark artifacts in this PR, along with some inconsistencies between the benchmark code and its comments. I suggest splitting the work: merge the low-risk deduplication and clone removal first, then fix the benchmark and provide PPSSPP/PSP arena_bump_bytes and layout timing results before evaluating the Taffy storage rebuild strategy separately.

For now, I’m requesting changes.

@quake

quake commented Sep 5, 2026

Copy link
Copy Markdown
Author

I have narrowed this PR to the two low-risk core changes:

  • keep the MeasureCtx data deduplication
  • keep the recursive traversal changes that avoid children.clone()
  • remove the Taffy capacity-rebuild strategy
  • remove the unrelated/incomplete benchmark artifacts

The Taffy storage rebuild will be proposed in a separate PR after adding PSP/PPSSPP arena_bump_bytes and layout timing measurements. This PR now passes the core test suite: 127 tests passed.

@quake
quake force-pushed the bench-optimize/sep3 branch from 96b3e96 to 553d0fd Compare September 5, 2026 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants