dchest / scrypt-async-js

Fast "async" scrypt implementation in JavaScript
http://dchest.github.io/scrypt-async-js/
BSD 2-Clause "Simplified" License
139 stars 26 forks source link

Reduce nextTick calls to prevent nextTick throttling #41

Closed Tschuck closed 6 years ago

Tschuck commented 6 years ago

With frequently called nextTick functions, it may be possible that the browser throttles the calls and the callback is only called after a long time. This delays the application. The pull request should reduce the number of nextTick calls and counteract this behavior.

View eth-lightwallet issue for a more detailed problem description.

dchest commented 6 years ago

Interesting. But why not use interruptStep for throttling?

interruptStep — (optional) the amount of loop cycles to execute before the next setImmediate/setTimeout (defaults to 0)

Tschuck commented 6 years ago

It's not a problem with the blocking of the browser, so we need to throttle the calculation. The problem is, that the throttling using nextTick will stop the browser responding to the nextTick call. Our current value of interruptStep is set to 200.

So i think its not a problem with setTimeout / setImmediate, but with the nextTick. Each run of the calculateAsync function, runs the nextTick function 3 times. After a huge amount of nextTick, the browser will throttle the nextTick requests of the application.

Or did i misinterpret something?

dchest commented 6 years ago

I think what causes this is that browsers throttle setTimeout (which is what nextTick is, unless the browser implements setImmediate). Initially setTimeout responds quickly, but if fired too much too quickly, perhaps it's throttled? Do you use a setImmediate shim, such as https://github.com/YuzuJS/setImmediate? If not, can you try reproducing with it? If yes, can you try reproducing without it? :)

dchest commented 6 years ago

BTW, what are other parameters (I think 200 for interruptStep is too small, so it causes a lot of nextTick very quickly)?

Tschuck commented 6 years ago

I missed the prefill of nextTick and setTimeout, i was a little confused...^^

Thanks for the interruptStep tip. I tried some tests with higher values from 1000 - 2000 and ended up with 10 - 20 setTimeouts, so everything seems to work as expected.

I will remove the pull request and make the changes to the lightwallet library.

Thanks for your support and sorry for the confusion :)

dchest commented 6 years ago

Awesome, thanks for figuring it out and posting here!

BTW, I also have a separate project with scrypt that automatically adjusts the time for next async:

https://www.stablelib.com/modules/_scrypt_scrypt_.html (see deriveKeyNonBlocking) (source: https://github.com/StableLib/stablelib/tree/master/packages/scrypt)

npm install @stablelib/scrypt

Maybe it will work well for such situations.

Tschuck commented 6 years ago

A automatic calculation would be awesome. Sadly I am a bit limited in my time for testing this at the moment.

I have made a pull request in the eth-lightwallet library. I'll put in the alternative as a hint for a improvement.

Thank you :)