perf: avoid reordering the range cache on every hit - #893
Open
jeet-dhandha wants to merge 1 commit into
Open
Conversation
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
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.
Range.parseRangeis memoized ininternal/lrucache.js. On every cache hit, the current implementation doesmap.delete(key)+map.set(key, value)to move the entry to the end of the insertion order. That reorder is pure overhead on a hot path.This replaces the single map with two generations and drops the reorder. A hit in the old generation is promoted back into the live one; when the live generation fills, it becomes the old one and the previous old generation is dropped.
parseRangeis deterministic, and the memo key already covers the range string plus both flags that affect parsing, so the eviction policy cannot change any result — only whether a given call re-parses. I verified that directly: 43,200 cases acrosssatisfies/validRange/maxSatisfying/minSatisfying/minVersion/intersects/subset, each under{},{loose},{includePrerelease}, and both, produce byte-identical output before and after.Benchmarks on node v24.8.0, one variant per process, best-of-5 within a process, min across 5 rounds. Corpus is every distinct range and version in the eslint / express / lodash / react / typescript / webpack packuments (1,965 ranges, 7,947 versions):
benchmarks/bench-satisfies.jsas writtenmaxSatisfyingover 7,900 versionsNo shape I tried is slower than stock. The retained ceiling goes from 1,000 entries (~0.78 MB) to 2,000 (~1.88 MB), measured by flooding 80,000 distinct ranges and sampling retained heap.
Two things worth flagging for review:
test/internal/lrucache.jsasserted that the oldest key is gone immediately after the first over-cap insert, which is specific to strict LRU. I replaced it with tests for the properties the cache actually needs: value round-trip,set(key, undefined)as a no-op,deleteclearing both generations, eviction under flood, and the2 * maxbound holding under promotion.For context on why the second generation is there rather than just deleting the reorder: dropping the reorder alone turns the cache into FIFO, which evicts a hot set that strict LRU would pin. On a hot set of 900 ranges just under the 1,000 cap, that regresses to 0.40x. The second generation is what avoids that.