perf(runtime): Map.clear early exit on empty maps; skip side-table probes for small numeric-keyed maps (ECS round 4) - #8938
Conversation
…side-tables for small numeric-keyed maps Every index js_map_clear resets mirrors the entries exactly, so an already-empty map has nothing to reset — the per-entity `adds.clear(); removes.clear()` of an ECS change set is that case half the time. And the string and pointer side-tables hold an entry only for a string or pointer key the map holds: for a small map whose keys are all bits-stable numerics, reading its keys is cheaper than the two thread-local resolutions plus two hash probes that find two empty tables, which was the whole cost of the ten thousand clears a frame on the grouping maps. Claude-Session: https://claude.ai/code/session_01FUvFrRNZyc5qknBiJbYbby
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesMap.clear optimization
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: ✨ 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 |
# Conflicts: # crates/perry-runtime/src/map.rs
|
Merged. Rebased onto current Validation: runtime 2770/0 ( |
|
Paired measurement (idle Mac mini, 9 alternating pairs, |
One runtime mechanism from the ECS round-4 chain, cut from current main. Measurement on the
codehz/ecs"5k entities: 3 commands each + sync" row (paired alternating runs on the idle Mac mini) is being taken against current main and will be posted here; suites: runtime (2745) incl.map::serial, on the isolated perrymaster gate.Map.clearreturns early on an empty map and skips the side-tables for small numeric-keyed maps. Every indexjs_map_clearresets mirrors the entries exactly (insert, delete, clear and the GC rewrites keep them in step), so an already-empty map has nothing to reset — the per-entityadds.clear(); removes.clear()of an ECS change set is that case half the time. And the string and pointer side-tables hold an entry only for a string or pointer key the map holds: for a map of at most 16 entries whose keys are all bits-stable numerics (is_safe_numeric_key), reading its keys is cheaper than the two thread-local resolutions plus two hash probes that find two empty tables — which the post-4h profile showed was the whole cost ofjs_map_clear(both hot offsets on its two TLS probes, ~10k clears a frame on the grouping maps, 1.1% of the frame). Larger maps and any string/pointer key reset the side-tables unconditionally as before.clear_resets_every_index_whatever_the_key_kindspins the numeric-only, empty, string-keyed and past-the-bound shapes (a stale content-hashed index entry would otherwise misroute a re-added key).https://claude.ai/code/session_01FUvFrRNZyc5qknBiJbYbby
Summary by CodeRabbit
Performance
Map.clear()performance, especially for frequently cleared small maps.Bug Fixes