Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 73a3d79

Browse filesBrowse files
jviidejviide
authored
fix: optimize Range parsing and formatting (#726)
<!-- What / Why --> <!-- Describe the request in detail. What it does and why it's being changed. --> This pull request optimizes the Range class in the following ways: 1. Produce fewer intermediate objects when reducing a range's space characters to single spaces. This seems to improve bench-subset scores by up to 5%, and bench-satisfies scores to a lesser degree. 2. Optimize Range formatting with explicit for loops instead, avoiding an intermediate array creation. This seems to improve bench-satisfies and bench-subset scores by up to 20%. 3. Calculate Range's `.range` string (used by `.format()` and `.toString()`) lazily. This seems to improve bench-satisfies and bench-subset scores by up to 9%. The external interface for the class stays the same, except for the new internal `.formatted` property used to cache its lazily calculated string. `Range#range` property is now also read-only. There is a new test lazy formatting to ensure full test coverage. The benchmarks bench-satisfies and bench-subset benefit from these changes, sometimes by up to 40%. Other benchmark results seem to stay the same. Here are the affected benchmarks before: ``` $ node benchmarks/bench-satisfies.js satisfies(1.0.6, 1.0.3||^2.0.0) x 695,094 ops/sec ±0.68% (97 runs sampled) satisfies(1.0.6, 2.2.2||~3.0.0) x 764,115 ops/sec ±0.40% (99 runs sampled) satisfies(1.0.6, 2.3.0||<4.0.0) x 805,593 ops/sec ±0.62% (97 runs sampled) satisfies(1.0.6, 1.0.3||^2.0.0, {"includePrelease":true}) x 695,045 ops/sec ±0.73% (95 runs sampled) satisfies(1.0.6, 2.2.2||~3.0.0, {"includePrelease":true}) x 750,433 ops/sec ±0.66% (99 runs sampled) satisfies(1.0.6, 2.3.0||<4.0.0, {"includePrelease":true}) x 787,903 ops/sec ±0.39% (99 runs sampled) satisfies(1.0.6, 1.0.3||^2.0.0, {"includePrelease":true,"loose":true}) x 652,166 ops/sec ±0.34% (99 runs sampled) satisfies(1.0.6, 2.2.2||~3.0.0, {"includePrelease":true,"loose":true}) x 696,377 ops/sec ±0.36% (96 runs sampled) satisfies(1.0.6, 2.3.0||<4.0.0, {"includePrelease":true,"loose":true}) x 721,729 ops/sec ±0.35% (98 runs sampled) satisfies(1.0.6, 1.0.3||^2.0.0, {"includePrelease":true,"loose":true,"rtl":true}) x 585,692 ops/sec ±0.75% (95 runs sampled) satisfies(1.0.6, 2.2.2||~3.0.0, {"includePrelease":true,"loose":true,"rtl":true}) x 631,653 ops/sec ±0.33% (96 runs sampled) satisfies(1.0.6, 2.3.0||<4.0.0, {"includePrelease":true,"loose":true,"rtl":true}) x 650,110 ops/sec ±0.64% (95 runs sampled) $ node benchmarks/bench-subset.js subset(1.2.3, *) x 633,342 ops/sec ±0.61% (95 runs sampled) subset(^1.2.3, *) x 743,036 ops/sec ±0.47% (97 runs sampled) subset(^1.2.3-pre.0, *) x 680,087 ops/sec ±0.76% (98 runs sampled) subset(^1.2.3-pre.0, *) x 680,948 ops/sec ±0.46% (96 runs sampled) subset(1 || 2 || 3, *) x 330,669 ops/sec ±0.53% (98 runs sampled) ``` And after: ``` $ node benchmarks/bench-satisfies.js satisfies(1.0.6, 1.0.3||^2.0.0) x 896,936 ops/sec ±0.53% (94 runs sampled) satisfies(1.0.6, 2.2.2||~3.0.0) x 998,214 ops/sec ±0.40% (95 runs sampled) satisfies(1.0.6, 2.3.0||<4.0.0) x 1,000,593 ops/sec ±0.43% (97 runs sampled) satisfies(1.0.6, 1.0.3||^2.0.0, {"includePrelease":true}) x 890,369 ops/sec ±0.41% (100 runs sampled) satisfies(1.0.6, 2.2.2||~3.0.0, {"includePrelease":true}) x 977,239 ops/sec ±0.48% (97 runs sampled) satisfies(1.0.6, 2.3.0||<4.0.0, {"includePrelease":true}) x 983,682 ops/sec ±0.95% (96 runs sampled) satisfies(1.0.6, 1.0.3||^2.0.0, {"includePrelease":true,"loose":true}) x 805,330 ops/sec ±0.84% (98 runs sampled) satisfies(1.0.6, 2.2.2||~3.0.0, {"includePrelease":true,"loose":true}) x 894,117 ops/sec ±0.43% (99 runs sampled) satisfies(1.0.6, 2.3.0||<4.0.0, {"includePrelease":true,"loose":true}) x 911,742 ops/sec ±0.42% (96 runs sampled) satisfies(1.0.6, 1.0.3||^2.0.0, {"includePrelease":true,"loose":true,"rtl":true}) x 741,254 ops/sec ±0.35% (97 runs sampled) satisfies(1.0.6, 2.2.2||~3.0.0, {"includePrelease":true,"loose":true,"rtl":true}) x 807,380 ops/sec ±0.42% (99 runs sampled) satisfies(1.0.6, 2.3.0||<4.0.0, {"includePrelease":true,"loose":true,"rtl":true}) x 820,390 ops/sec ±0.37% (99 runs sampled) $ node benchmarks/bench-subset.js subset(1.2.3, *) x 905,030 ops/sec ±0.63% (96 runs sampled) subset(^1.2.3, *) x 1,026,457 ops/sec ±0.63% (95 runs sampled) subset(^1.2.3-pre.0, *) x 923,789 ops/sec ±0.41% (97 runs sampled) subset(^1.2.3-pre.0, *) x 923,136 ops/sec ±0.44% (96 runs sampled) subset(1 || 2 || 3, *) x 432,037 ops/sec ±0.67% (96 runs sampled) ```
1 parent 2975ece commit 73a3d79
Copy full SHA for 73a3d79

File tree

Expand file treeCollapse file tree

2 files changed

+33
-10
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+33
-10
lines changed
Open diff view settings
Collapse file

‎classes/range.js‎

Copy file name to clipboardExpand all lines: classes/range.js
+24-10Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const SPACE_CHARACTERS = /\s+/g
2+
13
// hoisted class for cyclic dependency
24
class Range {
35
constructor (range, options) {
@@ -18,7 +20,7 @@ class Range {
1820
// just put it in the set and return
1921
this.raw = range.value
2022
this.set = [[range]]
21-
this.format()
23+
this.formatted = undefined
2224
return this
2325
}
2426

@@ -29,10 +31,7 @@ class Range {
2931
// First reduce all whitespace as much as possible so we do not have to rely
3032
// on potentially slow regexes like \s*. This is then stored and used for
3133
// future error messages as well.
32-
this.raw = range
33-
.trim()
34-
.split(/\s+/)
35-
.join(' ')
34+
this.raw = range.trim().replace(SPACE_CHARACTERS, ' ')
3635

3736
// First, split on ||
3837
this.set = this.raw
@@ -66,14 +65,29 @@ class Range {
6665
}
6766
}
6867

69-
this.format()
68+
this.formatted = undefined
69+
}
70+
71+
get range () {
72+
if (this.formatted === undefined) {
73+
this.formatted = ''
74+
for (let i = 0; i < this.set.length; i++) {
75+
if (i > 0) {
76+
this.formatted += '||'
77+
}
78+
const comps = this.set[i]
79+
for (let k = 0; k < comps.length; k++) {
80+
if (k > 0) {
81+
this.formatted += ' '
82+
}
83+
this.formatted += comps[k].toString().trim()
84+
}
85+
}
86+
}
87+
return this.formatted
7088
}
7189

7290
format () {
73-
this.range = this.set
74-
.map((comps) => comps.join(' ').trim())
75-
.join('||')
76-
.trim()
7791
return this.range
7892
}
7993

Collapse file

‎test/classes/range.js‎

Copy file name to clipboardExpand all lines: test/classes/range.js
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,15 @@ test('tostrings', (t) => {
8282
t.end()
8383
})
8484

85+
test('formatted value is calculated lazily and cached', (t) => {
86+
const r = new Range('>= v1.2.3')
87+
t.equal(r.formatted, undefined)
88+
t.equal(r.format(), '>=1.2.3')
89+
t.equal(r.formatted, '>=1.2.3')
90+
t.equal(r.format(), '>=1.2.3')
91+
t.end()
92+
})
93+
8594
test('ranges intersect', (t) => {
8695
rangeIntersection.forEach(([r0, r1, expect]) => {
8796
t.test(`${r0} <~> ${r1}`, t => {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.