Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid re-instantiating SemVer during diff compare #547

Merged
merged 1 commit into from
Apr 12, 2023

Conversation

macno
Copy link
Contributor

@macno macno commented Apr 12, 2023

Following #539 this small change improves speed by ~10% just calling compare directly on the semver object returned by parse

Performance on branch v7.4.0

╭─macno@cayenne ~/Projects/opensource/node-semver  ‹v7.4.0›
╰─➤  node benchmark.js
diff(0.0.1, 0.0.1) x 4,290,326 ops/sec ±0.34% (95 runs sampled)
diff(0.0.1, 0.0.1-pre) x 2,136,948 ops/sec ±0.06% (94 runs sampled)
diff(0.0.1, 0.0.1-pre-2) x 2,123,307 ops/sec ±0.12% (95 runs sampled)
diff(1.1.0, 1.1.0-pre) x 2,124,436 ops/sec ±0.12% (102 runs sampled)

And after the change

╭─macno@cayenne ~/Projects/opensource/node-semver  ‹main›
╰─➤  node benchmark.js
diff(0.0.1, 0.0.1) x 4,801,698 ops/sec ±0.30% (90 runs sampled)
diff(0.0.1, 0.0.1-pre) x 2,323,553 ops/sec ±0.06% (98 runs sampled)
diff(0.0.1, 0.0.1-pre-2) x 2,307,169 ops/sec ±0.14% (98 runs sampled)
diff(1.1.0, 1.1.0-pre) x 2,319,211 ops/sec ±0.11% (95 runs sampled)
benchmark.js
const Benchmark = require('benchmark')
const diff = require('./functions/diff')
const suite = new Benchmark.Suite()

const cases = [
  ['0.0.1', '0.0.1', null ],
  ['0.0.1', '0.0.1-pre', 'patch'],
  ['0.0.1', '0.0.1-pre-2', 'patch'],
  ['1.1.0', '1.1.0-pre', 'minor'],
]

for (const [v1, v2] of cases) {
  suite.add(`diff(${v1}, ${v2})`, function () {
    diff(v1, v2)
  })
}

suite
  .on('cycle', function (event) {
    console.log(String(event.target))
  })
  .run({ async: false })

@macno macno requested a review from a team as a code owner April 12, 2023 07:27
@macno macno requested a review from wraithgar April 12, 2023 07:27
@wraithgar wraithgar changed the title fix: faster diff avoiding twice instantiation of semver objects fix: avoid re-instantiating SemVer during diff compare Apr 12, 2023
@wraithgar wraithgar merged commit 2781767 into npm:main Apr 12, 2023
@github-actions github-actions bot mentioned this pull request Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants