fix: optimistic parse

[H4ad]

I saw these lines that check if the version is valid and I thought, what if we remove it?

node-semver/functions/parse.js

Lines 21 to 24 in f66cc45

	const r = options.loose ? re[t.LOOSE] : re[t.FULL]
	if (!r.test(version)) {
	return null
	}

Well, the perf before:

parse(1.2.1) x 2,282,718 ops/sec ±1.53% (94 runs sampled)
parse(1.2.2-4) x 917,890 ops/sec ±0.79% (88 runs sampled)
parse(1.2.3-pre) x 878,671 ops/sec ±1.07% (91 runs sampled)
invalid parse(90071992547409910.0.0) x 76,913 ops/sec ±0.97% (91 runs sampled)
invalid parse(hello, world) x 27,848,607 ops/sec ±2.58% (89 runs sampled)
invalid parse(xyz) x 28,323,638 ops/sec ±2.53% (88 runs sampled)

Then:

parse(1.2.1) x 2,758,186 ops/sec ±1.72% (94 runs sampled)
parse(1.2.2-4) x 990,634 ops/sec ±0.63% (93 runs sampled)
parse(1.2.3-pre) x 988,248 ops/sec ±1.12% (94 runs sampled)
invalid parse(90071992547409910.0.0) x 75,989 ops/sec ±1.10% (91 runs sampled)
invalid parse(hello, world) x 82,456 ops/sec ±0.74% (93 runs sampled)
invalid parse(xyz) x 83,357 ops/sec ±0.80% (92 runs sampled)

It massive slowdown the invalid cases because Semver throws a exception (which is very slow) and then return null.

But for optimistic cases, we can see a very good speedup.

If we combine this optimization with parseOptions, from:

parse(1.2.1) x 2,526,888 ops/sec ±0.98% (95 runs sampled)
parse(1.2.2-4) x 962,227 ops/sec ±2.39% (94 runs sampled)
parse(1.2.3-pre) x 970,360 ops/sec ±0.19% (93 runs sampled)
invalid parse(90071992547409910.0.0) x 76,354 ops/sec ±1.01% (90 runs sampled)
invalid parse(hello, world) x 29,528,801 ops/sec ±2.66% (93 runs sampled)
invalid parse(xyz) x 30,120,145 ops/sec ±2.43% (87 runs sampled)

We speedup up to:

parse(1.2.1) x 3,552,142 ops/sec ±2.85% (92 runs sampled)
parse(1.2.2-4) x 1,128,861 ops/sec ±0.52% (91 runs sampled)
parse(1.2.3-pre) x 1,109,481 ops/sec ±0.85% (96 runs sampled)
invalid parse(90071992547409910.0.0) x 76,316 ops/sec ±0.73% (90 runs sampled)
invalid parse(hello, world) x 82,178 ops/sec ±0.54% (96 runs sampled)
invalid parse(xyz) x 82,352 ops/sec ±0.77% (91 runs sampled)

The main question is, how often PNPM and NPM see bad package versions?

References

benchmark.js

const Benchmark = require('benchmark');
const parse = require('./functions/parse');
const { MAX_SAFE_INTEGER } = require('./internal/constants');
const suite = new Benchmark.Suite();

const cases = ['1.2.1', '1.2.2-4', '1.2.3-pre'];
const invalidCases = [`${MAX_SAFE_INTEGER}0.0.0`, 'hello, world', 'xyz']

for (const test of cases) {
  suite.add(`parse(${test})`, function () {
    parse(test);
  });
}

for (const test of invalidCases) {
  suite.add(`invalid parse(${test})`, function () {
    parse(test);
  });
}

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

[H4ad]

@jakebailey Do you have some thoughts on this since you already had some experience benchmarking PNPM?

[wraithgar]

This is what the SemVer constructor is doing. As you can see it's not even consistent with this code in that SemVer trim()s the version.

    const m = version.trim().match(options.loose ? re[t.LOOSE] : re[t.FULL])

    if (!m) {
      throw new TypeError(`Invalid Version: ${version}`)
    }

[H4ad]

Yep, that's why I removed it, the idea in that test is just to fail-fast.

So, can we consider that a bug or we could try to push this change?

[wraithgar]

I think it's evidence in favor of this change. We don't need to optimize for bad semver here, and this PR is also a code cleanup and consistency issue.

[jakebailey]

Testing on pnpm, I don't see much difference, which doesn't surprise me given pnpm doesn't really call semver.parse.

FWIW, if you want to test the big pnpm DT case, here are some instructions:

• Clone https://github.com/jakebailey/DefinitelyTyped/tree/pnpm-workspaces-working
• Clone pnpm
• In pnpm, run: pnpm install && pnpm run compile && ./pnpm/dev/pd.js --version > /dev/null.
  • This is the fastest way to getting a local build in my experience. pd.js is a wrapper which esbuilds pnpm then runs the output, so you can run it with a dummy flag to cause a build, then use its output file, dist/pnpm.cjs.
• In DT, run node /path/to/pnpm/pnpm/dev/dist/pnpm.cjs install to populate pnpm's cache.
• In DT, run git clean -fdx to clear the repo, then node /path/to/pnpm/pnpm/dev/dist/pnpm.cjs install --offline to perform an install using the cached store.

Then to do timing + memory stats, I have a timing.sh with:

#!/usr/bin/zsh

if [[ `uname` == Darwin ]]; then
    MAX_MEMORY_UNITS=KB
else
    MAX_MEMORY_UNITS=MB
fi

TIMEFMT=\
'total time:  %E'$'\n'\
'user time:   %U'$'\n'\
'system time: %S'$'\n'\
'CPU percent: %P'$'\n'\
'max memory:  %M '$MAX_MEMORY_UNITS''$'\n'

time $@

So you can do timing.sh node /path/to/pnpm/pnpm/dev/dist/pnpm.cjs install --offline to get stats.