Fix excessive memory allocation described in #4 #5
Conversation
implementation argon2-jvm.
This looks more object-oriented and potentially reduces the number of pointer dereferences during bulk computation.
|
This is probably a tough PR to get merged seeing as this also includes changing from Maven to Gradle as well as many unrelated changes (rephrasing stuff in the readme, ...). |
|
|
||
| private boolean clearMemory = true; | ||
| private Charset charset = Charset.forName("UTF-8"); | ||
| private static Charset charset = Charset.forName( "UTF-8" ); |
There was a problem hiding this comment.
The spaces preceding/following the brackets are really uncommon in Java.
There was a problem hiding this comment.
I totally agree. Adding an Eclipse auto-format save action to the project would help mimimizing those issues.
We (@dbusche and I) started evaluating, benchmarking and finally fixing the code from the latest version committed by @minebreaker, since it looked like a straight forward advancement (including hash encoding and cleanup). Don't know, why he did not post a pull-request. Yes, the readme was adjusted, since @minebreaker moved to Java 8, removed unneccessary dependencies for the command-line version (which I think is really not required for a crypto lib), and finally added this really usefull hash-encoding stuff. Therefore I consolidated the documentation reflecting the latest state. Yes, these changes are "unrelated" to the memory issue but If we started from the original @andreas1327250 code, the really useful @minebreaker stuff would have probably become unmergable. |
4 participants
As described in #4 the hash computation allocates far to much temporary memory resulting in multiple GC runs per call. Especially f2d8b33 is critical to make the implementation usable in a multi-user environment.