Skip to content

fix: faster toString for integrity#75

Merged
wraithgar merged 1 commit intonpm:mainfrom
h4ad-forks:fix/faster-to-string
Apr 3, 2023
Merged

fix: faster toString for integrity#75
wraithgar merged 1 commit intonpm:mainfrom
h4ad-forks:fix/faster-to-string

Conversation

@H4ad
Copy link
Contributor

@H4ad H4ad commented Apr 3, 2023

Performance before:

parsed.toString() x 1,595,186 ops/sec ±1.35% (90 runs sampled)
parsedStrict.toString() x 1,553,016 ops/sec ±1.82% (90 runs sampled)

After:

parsed.toString() x 8,128,007 ops/sec ±1.82% (90 runs sampled)
parsedStrict.toString() x 8,322,286 ops/sec ±1.30% (93 runs sampled)
benchmark.js
const Benchmark = require('benchmark')
const ssri = require('./lib/index');
const suite = new Benchmark.Suite;

const integrity = `sha512-WrLorGiX4iEWOOOaJSiCrmDIamA47exH+Bz7tVwIPb4sCU8w4iNqGCqYuspMMeU5pgz/sU7koP5u8W3RCUojGw== sha256-Qhx213Vjr6GRSEawEL0WTzlb00whAuXpngy5zxc8HYc=`;
const parsed = ssri.parse(integrity);
const parsedStrict = ssri.parse(integrity, { strict: true });

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

When parsing is strict, I introduce a small break change because I know the possible hashes, so I call it directly.
Instead of the toString being stringified in the order in which the hashes were added, it will always be stringified in this order: sha512, sha384 and sha256.

If you think that is problematic, we can keep the loop, even if is a little bit slower.

References

Related to #71

@H4ad H4ad requested a review from a team as a code owner April 3, 2023 15:43
@H4ad H4ad requested a review from wraithgar April 3, 2023 15:43
@wraithgar
Copy link
Member

npm run lintfix should shore up the linting errors (it usually fixes everything but line length warnings)

@H4ad H4ad force-pushed the fix/faster-to-string branch from 3479114 to ddb5e82 Compare April 3, 2023 16:51
@wraithgar
Copy link
Member

Reading the readme again we do not make any guarantees about the order that the algorithms will be in toString. The order doesn't matter to things like parse either so I think this can remain a patch release.

@H4ad H4ad force-pushed the fix/faster-to-string branch from ddb5e82 to 44078c0 Compare April 3, 2023 21:01
@wraithgar wraithgar merged commit a316b12 into npm:main Apr 3, 2023
@github-actions github-actions bot mentioned this pull request Apr 3, 2023
@H4ad H4ad deleted the fix/faster-to-string branch April 6, 2023 17:00
H4ad added a commit to h4ad-forks/ssri that referenced this pull request Dec 15, 2025
@H4ad H4ad mentioned this pull request Dec 15, 2025
wraithgar pushed a commit that referenced this pull request Dec 16, 2025
Reference:
- #71
- #72
- #75
- #76

Started by #160
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