Perf improvements for sha1.js, sha2.js, and sha2_64b.js #322
Conversation
allocating additional memory (and incurring additional garbage collections) unnecessarily. ~10% speed gain on Chrome 35. goog/crypt/sha2.js: Port the block-size fast path from Sha1.update to Sha2.update. Compute message schedule on the fly. Remove code for testing for invalid inputs that is not present in Sha1.update. ~40% speed gain on Chrome 35. goog/crypt/sha256_test.html: Remove testBadInput(); no similar test is performed in sha1_test.html
| // Applied optimization from sha1.js | ||
| if (inbuf == 0) { | ||
| while (n <= lengthMinusBlock) { | ||
| this.computeChunk_(bytes, n); |
There was a problem hiding this comment.
computeChunk_ takes no arguments. It computes over this.chunk_ which should be a block-sized array containing the current message chunk to update. With this change this.chunk_ doesn't appear to be updated when iterating over the full blocks so the hash is not computed properly for messages which are the same size or larger than one block. Amazingly the tests still pass(!).
…repo. closure/goog/crypt/sha256_test.js: Add some KATs.
|
Indeed they do! Fixed that by adding better tests. Calling style now identical to SHA1, tests pass, might be correct even. Also fixed a bug in sha1.js when input length is explicitly passed but longer than input. Synchronized fix to sha2.js. |
…message than the message's length. closure/goog/crypt/sha1_test.js: Tests for this. closure/goog/crypt/sha256_test.js: Rename tests, synchronize with sha1_test.js.
…recomputed message schedules. goog/crypt/sha256_test.js: Added tests for precomputed message schedule case; cleaned up other tests somewhat.
|
Just as a note: The present client code for the precomputed schedule code (the new functions prescheduled and scheduledUpdate) is End-to-End’s S2K: https://github.com/coruus/e2e/blob/s2k-sha256/javascript/crypto/e2e/openpgp/s2k.js |
new ones repeatedly; update message schedule in-place. 20% performance gain, Chrome 36.
…, SHA2-384, and SHA2-512. (Note: this test takes a few minutes.) shamcs.py: The Python script used to generate the KATs.
coruus
changed the title
|
Added Monte Carlo known-answer-tests for all the SHA functions in Closure. (The script shamc.py was used to generate them, if you would like to choose different parameters.) Improved performance of SHA-2-512 by about 20% on Chrome 36 by not allocating unneeded goog.math.Longs. All tests pass, including new KATs. |
|
Thanks for working on this. I'm on vacation right now, but I'll have a look when I return next week. |
|
|
||
| // TODO(user): Try to unroll this loop to speed up the computation. | ||
| for (var i = 0; i < 80; i++) { | ||
| var t = (((a << 5) | (a >>> 27)) + f + e + k + W[i]) & 0xffffffff; |
There was a problem hiding this comment.
Since you're modifying this file, please change "& 0xffffffff" to "| 0" here and elsewhere.
|
I ran some performance tests of my own: closure-optimized code which computes hash of a 3MB string and also computes many hashes of short strings. The results look good for sha1 and sha256. For Chrome 37: sha1 and sha256 are ~20-30% faster for the large string tests, and maybe 10% for the short string tests. On Firefox 31, sha256 is ~6x faster for sha256 large string test and 20% faster for sha1 large string test. Somehow this code fixes some performance regression with sha256 on firefox which has crept in. The sha*_perf.html tests show comparable results. SHA512 seems to be slower, though. ~50% slower on Chrome 37. So all-in-all the optimizations added to speed up update() in sha1.js and sha2.js look good to me. Very nice improvement :-) I'll let the closure library maintainers comment on the style and the appropriateness of extending the api to include preschedule() and scheduledUpdate() as well as the new tests. |
|
Thanks much! (I don't know how I fooled myself into thinking I had improved sha2_64b.js; my baseline run must have been way way off. That's been reverted.) |
…ting boundary-value test cases. closure/goog/crypt/sha256_test.js: Added test-case for 54-byte message.
| W[i] = ((buf.charCodeAt(opt_offset) << 24) | | ||
| (buf.charCodeAt(opt_offset + 1) << 16) | | ||
| (buf.charCodeAt(opt_offset + 2) << 8) | | ||
| (buf.charCodeAt(opt_offset + 3))) | 0; |
There was a problem hiding this comment.
The "| 0" is a type hint to the VM? Can you note this in a comment at the top of the file?
…cases. Fix sha2 tests to properly cover multi-block messages. ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=71869454 Conflicts: closure/goog/crypt/sha256_test.js
…2_32b Conflicts: closure/goog/crypt/sha256_test.js closure/goog/deps.js
|
Any updates on this? |
|
I'm not quite clear on jacquerie's comments here? Is there something that I should be doing (dlg-yahoo == coruus). |
|
Err, that was a major screwup on my part. I mistakenly force pushed on my own fork of google/closure-library a copy of google/traceur-compiler. This resulted in the creation of hundreds of comments referring to a completely different project. I asked GitHub if there's anything I can do to fix this, they said no. I'm terribly sorry! |
|
@meheffernan Where are we at here? |
|
It looks good to me. The performance improvement is nice especially on FF. |
|
Merge conflicts must be resolved |
|
There is a rebased and squashed version at #391 @meheffernan Could I get some feedback before I need to rebase again? |
|
@dig-yahoo As I mentioned on Nov 13, it looks good to me. Not sure if I'm supposed to explicitly approve it in the UI or not (I don't know github). FWIW I just verified the performance results with the latest change on FF 34.0 and Chrome 40.0. Looks good. |
allocating additional memory (and incurring additional garbage
collections) unnecessarily. ~10% speed gain on Chrome 35.
goog/crypt/sha2.js: Port the block-size fast path from Sha1.update
to Sha2.update. Compute message schedule on the fly. Remove code
for testing for invalid inputs that is not present in Sha1.update.
~30% speed gain on Chrome 35.
goog/crypt/sha256_test.html: Remove testBadInput(); no similar test
is performed in sha1_test.html