search

StableLib / stablelib

A stable library of useful TypeScript/JavaScript code
https://www.stablelib.com
Other
175 stars 36 forks source link

Scrypt constructor should cache tmp array as well #2

Closed palant closed 7 years ago

palant commented 7 years ago

Right now, the tmp array is created on every smix() call. While that's only 64 bytes, causing memory churn here is unnecessary - this array can be cached in the Scrypt constructor, same as XY and V arrays.

dchest commented 7 years ago

I think I benchmarked it, and it turned out to be slower, but maybe it's false memory. Maybe allocation sinking in JS engines took care of it.

palant commented 7 years ago

At least in my benchmarking I didn't see any significant difference.

dchest commented 7 years ago

Good, thanks! I also benchmarked it today (with N=8, r=8, p=500, since it allocates once per p), and saw no difference in Node 8, so I think this allocation is sinked by V8. Let's keep it as it is, since this would be an unnecessary optimization. (I suspect its locality to the function will in fact help, not make it worse.)

palant commented 7 years ago

I actually meant that there is no performance difference, I'm not really convinced that there is no impact on memory.