perf: cache json initialization

[H4ad]

I was doing some perf for npm run and I found the sortTranspositions was taking 5ms to run.

So, I changed the code to be like spdx-license-ids and generated a JSON with the calculated transpositions and also with the last-resorts.

I also lazy load the dependencies to only import when needed, so we do not introduce any perf penalty for those who import this module but will not use it immediately.

In this way, we remove one dependency and we also improve the initialization of this module:

Before:

$ hyperfine --warmup 3 "node ./index.js"                                                                                                                                                            [22:38:45]
Benchmark 1: node ./index.js
  Time (mean ± σ):     109.3 ms ±   1.1 ms    [User: 95.4 ms, System: 38.1 ms]
  Range (min … max):   107.2 ms … 111.9 ms    27 runs

After:

$ hyperfine --warmup 3 "node ./index.js"                                                                                                                                  [22:36:37]
Benchmark 1: node ./index.js
  Time (mean ± σ):     101.0 ms ±   1.1 ms    [User: 88.5 ms, System: 34.1 ms]
  Range (min … max):    99.2 ms … 103.6 ms    29 runs

[kemitchell]

Are there performance concerns reported upstream? Or is this optimization for its own sake?

Removing dependency on spdx-license-ids may work a small load-time improvement, but it will also prevent relatively frequent updates to that package from flowing through to this package via npm upgrade, without lockstep releases of spdx-correct. In npm in particular, I expect there are also other edges to spdx-license-ids in the dependency tree, so essentially vendoring it under spdx-correct and continuing to depend on it elsewhere will lead to duplication in installs and distributed build archives.

I may have fallen behind on changes to the various npm subcommands. My impression was that npm continues to use this package for npm init, to help users choose valid package.json license property values. If the load time is a concern, why is npm run loading this package at all?

[H4ad]

Are there performance concerns reported upstream? Or is this optimization for its own sake?

There is no concern upstream, I'm just a dev with free time and willing to speedup npm run.

but it will also prevent relatively frequent updates to that package from flowing through to this package via

That's a valid concern, but I think you can solve this by using dependabot to be always updated with the latest version, to save some ms from the initialization, I think it is worth it.

In npm in particular, I expect there are also other edges to spdx-license-ids

You are right on this one:

npm@10.5.1 /home/h4ad/Projects/opensource/performance-analysis/npm-cli
├─┬ init-package-json@6.0.2
│ └─┬ validate-npm-package-license@3.0.4
│   └─┬ spdx-correct@3.2.0
│     └── spdx-license-ids@3.0.17 deduped
└─┬ spdx-expression-parse@3.0.1
  └── spdx-license-ids@3.0.17

If the load time is a concern, why is npm run loading this package at all?

Good question, from what I saw, is just for legacy purposes, so I will try to lazy load the initialization of this module.

But currently, importing this module will have a penalty of 5ms without even calling the exported function.

---

In case you don't want to ship the JSONs, I will totally understand! If so, can we at least lazy load the initialization to when this module is being called? I think this will be the ideal world, to leave the penalty only when this module is actually being used.

[kemitchell]

If you're hunting perf gains, I'm guessing the real prize is in npm. I don't know what legacy npm run preserves by loading this package, but I maintain this package, and I have no idea what npm run has to do with it!

As for this package alone, I'm disinclined to make a chore of lockstep releases, especially premature of any perf complaints. It's not a pure win, given how often this package gets bundled with other spdx-related packages. I'm not if this is being used in any Web client-side apps, but space matters, too.

Frankly, mostly it's my time that I value. I try to avoid making chores for myself.

[H4ad]

That's okay, totally valid reasons.

Before closing this PR, do you think is valid to create a PR to lazy load the initialization of transpositions and lastResorts?

[kemitchell]

I'm not exactly sure I know what you mean. But if you're feeling inspired to PR while you've got it fresh in mind, I'll earnestly review your proposal. Maybe not this evening—it's 1930 here—but when I can.

[H4ad]

I created the PR at #44

Take your time, I really appreciate your review, as a perf guy, I just look at the perf gains without thinking much about the other "costs" of the code.

And for me it's almost midnight, so I'm going to rest now ahahha