From 73a3d79c4ec32d5dd62c9d5f64e5af7fbdad9ec0 Mon Sep 17 00:00:00 2001 From: jviide Date: Tue, 16 Jul 2024 18:42:37 +0300 Subject: [PATCH] fix: optimize Range parsing and formatting (#726) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) ``` --- classes/range.js | 34 ++++++++++++++++++++++++---------- test/classes/range.js | 9 +++++++++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/classes/range.js b/classes/range.js index 117b45a2..ceee2314 100644 --- a/classes/range.js +++ b/classes/range.js @@ -1,3 +1,5 @@ +const SPACE_CHARACTERS = /\s+/g + // hoisted class for cyclic dependency class Range { constructor (range, options) { @@ -18,7 +20,7 @@ class Range { // just put it in the set and return this.raw = range.value this.set = [[range]] - this.format() + this.formatted = undefined return this } @@ -29,10 +31,7 @@ class Range { // First reduce all whitespace as much as possible so we do not have to rely // on potentially slow regexes like \s*. This is then stored and used for // future error messages as well. - this.raw = range - .trim() - .split(/\s+/) - .join(' ') + this.raw = range.trim().replace(SPACE_CHARACTERS, ' ') // First, split on || this.set = this.raw @@ -66,14 +65,29 @@ class Range { } } - this.format() + this.formatted = undefined + } + + get range () { + if (this.formatted === undefined) { + this.formatted = '' + for (let i = 0; i < this.set.length; i++) { + if (i > 0) { + this.formatted += '||' + } + const comps = this.set[i] + for (let k = 0; k < comps.length; k++) { + if (k > 0) { + this.formatted += ' ' + } + this.formatted += comps[k].toString().trim() + } + } + } + return this.formatted } format () { - this.range = this.set - .map((comps) => comps.join(' ').trim()) - .join('||') - .trim() return this.range } diff --git a/test/classes/range.js b/test/classes/range.js index 9547e23a..c1e6eb1b 100644 --- a/test/classes/range.js +++ b/test/classes/range.js @@ -82,6 +82,15 @@ test('tostrings', (t) => { t.end() }) +test('formatted value is calculated lazily and cached', (t) => { + const r = new Range('>= v1.2.3') + t.equal(r.formatted, undefined) + t.equal(r.format(), '>=1.2.3') + t.equal(r.formatted, '>=1.2.3') + t.equal(r.format(), '>=1.2.3') + t.end() +}) + test('ranges intersect', (t) => { rangeIntersection.forEach(([r0, r1, expect]) => { t.test(`${r0} <~> ${r1}`, t => {