perf(core): PHPMNT-394 Index sibling maps by value for constant-time lookup - #69
Draft
TomA-R wants to merge 1 commit into
Draft
perf(core): PHPMNT-394 Index sibling maps by value for constant-time lookup#69TomA-R wants to merge 1 commit into
TomA-R wants to merge 1 commit into
Conversation
TomA-R
force-pushed
the
phpmnt-394-index-sibling-lookups
branch
from
August 13, 2026 10:35
998140f to
99cb6dc
Compare
…lookup Every cache lookup scanned the sibling values one by one with isEqual, which is fine for a handful of distinct arguments but degrades badly once a level accumulates many of them (cycling calls through a large cache could take tens of seconds). Add a Map index per level of the key tree so most lookups become a single Map.get. Map key equality (identity, plus NaN equals NaN) matches the default comparison for everything except shallowly-equal- but-distinct objects, so: - objects that are shallowly equal but not the same instance fall back to the existing linear scan - a custom isEqual could treat values as equal that a Map would keep apart, so the index is disabled entirely in that case - levels with 8 or fewer siblings (MAX_SIBLINGS_FOR_SCAN) just keep scanning, since a Map lookup costs more than the one or two === checks a narrow scan needs; benchmarking confirms dropping this threshold regresses the most common memoize case (repeated calls with the same arguments) by 20-30% despite helping wider cases The lookup itself lives in a single _findMap helper (try the index, else scan) instead of a nested loop with a mutable isMatched flag, and the scan uses a manual for loop rather than findIndex(closure) to avoid allocating a closure on every call in this hot path. No behavioral differences: verified against the existing suite, a manual regression script covering NaN/objects/functions/LRU eviction/ multi-level lookups/custom isEqual, and by tracing real usage in checkout-sdk-js and checkout-js (both mostly use memoizeOne, which rarely engages this path, and the one custom-isEqual call site disables the index outright).
TomA-R
force-pushed
the
phpmnt-394-index-sibling-lookups
branch
from
August 23, 2026 12:09
99cb6dc to
a05eb8e
Compare
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.
Jira: PHPMNT-394
What/Why?
Looking up a cached value scans every sibling argument one by one. That's fine for a small cache, but it gets slow once a cache holds many entries, cycling 200k calls through 1000 cached keys took over a second.
This adds a
Mapindex next to the existing list, so most lookups become instant instead of scanning every entry.A couple of notes on how it stays correct:
Mapcan't find those by shape), so that case is unchanged.isEqualis passed in, the index is turned off entirely. A custom comparison might treat values as equal that aMapwouldn't.Map. I tried removing this cutoff and it made the most common case (repeated calls with the same arguments) 20-30% slower, so I kept it.Numbers (best of several runs):
Rollout/Rollback
Merge / revert
Testing
Existing tests pass, plus new tests for the new code paths (large caches,
NaN, functions, multi-argument lookups, customisEqual). No behavior changes: checked with a manual regression script and by reviewing real usage in checkout-sdk-js and checkout-js🤖 code changes made with help from Claude 🤖