Repository navigation
fix: 1.3–1.6x faster satisfies() with includePrerelease by not re-parsing comparator versions - #923
Open
deadstrobe5 wants to merge 1 commit into
Open
deadstrobe5 wants to merge 1 commit into
deadstrobe5 wants to merge 1 commit into
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.
satisfies()/maxSatisfying()with{ includePrerelease: true }get 1.3–1.6x faster, with identical results.Why
Comparator#parsecreates itsSemVerwiththis.options.looseinstead ofthis.options, so the comparator's version always hasincludePrerelease: false. When a Range is built with{ includePrerelease: true },Comparator#test→cmp→comparecallsnew SemVer(this.semver, options); theincludePrereleaseflags don't match, so the version string is re-parsed with the FULL regex on every test call. In a CPU profile,SemVerconstruction was ~24% and the version regex ~8% ofsatisfiestime.Passing the full options makes the instance reusable. Parsing only depends on
loose, which is unchanged, so results are identical. The only visible difference is thatcomparator.semver.includePrerelease/.optionsnow match the comparator's own options.Benchmark
Benchmark.js, median of 5 alternating runs per side, Node 26, Apple M5,
{ includePrerelease: true }:satisfies(1.0.6, '1.0.3||^2.0.0')satisfies(2.2.3, '1.0.3||^2.0.0')satisfies(3.0.1-beta.1, '2.2.2||~3.0.0')maxSatisfying(20 versions, '^1.0.0')On a corpus of 2,000 versions × 300 ranges:
satisfies~25% faster,maxSatisfying~41% faster. WithoutincludePrerelease: no change.(The existing
benchmarks/bench-satisfies.jsspells the optionincludePrelease, so it never hits this path; happy to fix that separately.)Tests
test/classes/comparator.jsthat the comparator'ssemvercarries the comparator options.npm testpasses with 100% coverage; eslint clean.satisfies,maxSatisfying/minSatisfying,minVersion,validRange,intersects,subset,gtr/ltr/outside,simplifyRange,toComparators, andRange/Comparator#testwith SemVer objects built under everyloose/includePrereleasecombination, plus leading zeros,v/=prefixes, build metadata and huge numbers): 0 differences, including error messages.Thanks for semver!