Closed timboudreau closed 6 years ago
@timboudreau, apparently github is no longer fully functional unless one is using latest browsers. At the moment I can not update to the latest OS X and it turns out there is not a single browser out there for 10.8.n that satisfies the UX requirements of the github boys.
So, while I fully understand what you are adding, I can review the details at this time. Nevertheless, thank you for the PL; I'll review and ack/nack at a future date.
s/can review/can't review
There's always git difftool
:-)
Good to know! I'll give it a whirl.
Greetings @timboudreau Finally have some time to take a look at your contribution.
My main concern on reviewing the code was possible performance hit. I have cloned your repo and spot checked a few benchmarks. Not sure if you checked the performance impact on your end, but there is an order of magnitude+ performance hit (from ~200Mb/s
to ~15Mb/s
throughput) with the introduction of state
in the critical compress loop.
Try
java -cp target/ove.blake2b-alpha.0-mlo.jar ove.crypto.digest.Bench -n <data size>
vs
java -cp target/ove.blake2b-alpha.0.jar ove.crypto.digest.Bench -n <data size>
The feature is quite nice but given the performance impact, I can't accept this PR as is. Will give it some thought on my end to see if the perf. hit can be mitigated. (I do thank you for your contribution, which is appreciated.)
Of the top of my head, likely getting state into a local variable so uses of it are not dereferencing an instance field might help.
In our case it was used to hash partial chunked uploads as they arrive, rather than iterating what could be a very large file later.
Yes, I tried that a bit earlier after closing this and recovered the perf. Rather sad day for javac
.
https://gist.github.com/alphazero/7f6737d93fa2cd322cb54ef33900a1ca
Regarding the use-case, is not the tree
mode giving you this and more?
Adds support for extracting the internal state of a hash operation in progress, such that it could be serialized to JSON or Java serialization, and later reloaded and continued (dependent upon passing in a Param identical to the original) - useful if you're implementing resumable uploads and storing data using its hash as URL path.