test.check's random number ClojureScript Implementation has a slow path in goog.math.Long's multiply method. The conditional checks (https://github.com/google/closure-library/blob/master/closure/goog/math/long.js#L683-L711) don't affect the correctness of the implementation. These conditionals instead add extra branching and allocation overhead.
This patch ports the goog.math.Long's multiply over to ClojureScript with the conditionals removed. My benchmarking (benchmark code here https://gist.github.com/spinningtopsofdoom/7fb7fbe4ae7de159922a79e6f3f3e730) saw around a 2X speedup of Long generation
I've ported of Google Closure Libraries test of the goog.math.Long multiply method to ClojureScript in this gist (https://gist.github.com/spinningtopsofdoom/e479bf6f3485cd85ba979b4e5562057a).
This is a good catch, thanks!
This kind of change is a good candidate for a property-based test (comparing your implementation and .multiply to check that they return the same values) – I can wait for you to add that to the patch if you'd like, or apply the patch and write it myself.
I can add that to the patch. Since .multiply is the current standard would this the property based test would need to use the .multiply based PRNG? Also where would you like this test to be located?
The new patch added the test to clojure.test.check.test and used the new test.check multiply generator.
Thanks, this looks good. I'm just trying to reproduce the speedup (which is taking me days just due to being busy) and then will apply the patch.
Applied on master, thanks!