Skip to content

perf: tiny improvement in scrypt #122

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.

Sign up for GitHub

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

Jump to bottom
Merged
paulmillr merged 3 commits into from
Sep 19, 2025
Merged

perf: tiny improvement in scrypt #122

paulmillr merged 3 commits into from
Sep 19, 2025
+4 −2

Conversation

@ChALkeR
Copy link
Contributor

@ChALkeR ChALkeR commented Sep 18, 2025 •
edited
Loading

I see an ~4% improvement from not using % N inside loop over N (and casting to int32)

N is always a power of two, as checked above

@ChALkeR ChALkeR force-pushed the chalker/perf/scrypt/0 branch 2 times, most recently from 02e45c7 to aff56ff Compare September 18, 2025 12:19
@ChALkeR ChALkeR force-pushed the chalker/perf/scrypt/0 branch from aff56ff to 15ee761 Compare September 18, 2025 12:27
@ChALkeR ChALkeR mentioned this pull request Sep 18, 2025
Draft
@paulmillr
Copy link
Owner

paulmillr commented Sep 18, 2025

make sure to add a brief code comment describing why this works

@paulmillr
Copy link
Owner

paulmillr commented Sep 18, 2025

also, did you check safari?

@ChALkeR
Copy link
Contributor Author

ChALkeR commented Sep 18, 2025 •
edited
Loading

@paulmillr I see similar 4-5% improvement on WebKit (in playwright on mac)
Had to fix benchmark code to check that though
Tested only on scrypt(pass, salt, { N: 2 ** 10, r: 8, p: 1, dkLen: 32 })

@ChALkeR
Copy link
Contributor Author

ChALkeR commented Sep 19, 2025 •
edited
Loading

Approximate:

Engine n = 2 ** 10 n = 2 ** 19
Node.js 22 505 → 520 (+2.84%) 0.95 → 0.97 (+3.03%)
Chromium 547 → 571 (+4.16%) 1.10 → 1.10 (+2.67%)
WebKit 447 → 477 (+6.99%) 0.82 → 0.88 (+7.50%)
Firefox 314 → 325 (+3.33%) 0.61 → 0.65 (+5.14%)
JSC 450 → 479 (+6.05%) 0.83 → 0.88 (+6.06%)
Raw data

Node.js, main:

scrypt(n: 2 ** 10, r: 8, p: 1) x 505 ops/sec @ 1978μs/op (1888μs..2ms)
scrypt(n: 2 ** 19, r: 8, p: 1) x 0.95 ops/sec @ 1057ms/op

Node.js, pr:

scrypt(n: 2 ** 10, r: 8, p: 1) x 520 ops/sec @ 1922μs/op (1771μs..11ms)
scrypt(n: 2 ** 19, r: 8, p: 1) x 0.97 ops/sec @ 1025ms/op

Chromium, main:

scrypt(n: 2 ** 10, r: 8, p: 1) x 547 ops/sec @ 1826μs/op (1600μs..8ms)
scrypt(n: 2 ** 19, r: 8, p: 1) x 1.1 ops/sec @ 938ms/op

Chromium, pr:

scrypt(n: 2 ** 10, r: 8, p: 1) x 571 ops/sec @ 1750μs/op (1600μs..8ms)
scrypt(n: 2 ** 19, r: 8, p: 1) x 1.1 ops/sec @ 913ms/op (905ms..922ms)

WebKit, main:

scrypt(n: 2 ** 10, r: 8, p: 1) x 447 ops/sec @ 2ms/op (2ms..3ms)
scrypt(n: 2 ** 19, r: 8, p: 1) x 0.82 ops/sec @ 1216ms/op

WebKit, pr:

scrypt(n: 2 ** 10, r: 8, p: 1) x 477 ops/sec @ 2ms/op (2ms..3ms)
scrypt(n: 2 ** 19, r: 8, p: 1) x 0.88 ops/sec @ 1131ms/op

Firefox, main

scrypt(n: 2 ** 10, r: 8, p: 1) x 314 ops/sec @ 3ms/op (2ms..7ms)
scrypt(n: 2 ** 19, r: 8, p: 1) x 0.61 ops/sec @ 1634ms/op

Firefox, pr:

scrypt(n: 2 ** 10, r: 8, p: 1) x 325 ops/sec @ 3ms/op (2ms..6ms)
scrypt(n: 2 ** 19, r: 8, p: 1) x 0.65 ops/sec @ 1550ms/op

jsc, main:

scrypt(n: 2 ** 10, r: 8, p: 1) x 450 ops/sec @ 2ms/op
scrypt(n: 2 ** 19, r: 8, p: 1) x 0.83 ops/sec @ 1204ms/op

jsc, pr:

scrypt(n: 2 ** 10, r: 8, p: 1) x 479 ops/sec @ 2ms/op
scrypt(n: 2 ** 19, r: 8, p: 1) x 0.88 ops/sec @ 1131ms/op

@paulmillr paulmillr merged commit 59fda2c into paulmillr:main Sep 19, 2025
6 checks passed
@paulmillr
Copy link
Owner

paulmillr commented Sep 19, 2025

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ChALkeR @paulmillr