RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

[Alan Bateman]

On Fri, 18 Dec 2020 13:27:02 GMT, PROgrm_JARvis <github.com+7693005+JarvisCraft at openjdk.org> wrote:

>> Are you planning to add a microbenchmark to demonstrate the issue?
>> If there is a holder class for the MessageDigest then its initialisation can fail, this would allow the orThrow method to go away
>
>> If there is a holder class for the MessageDigest then its initialisation can fail, this would allow the orThrow method to go away
> 
> As of allowing `Md5Digest` instead of explicitly throwing the exception from `orThrow` I think that the latter is more appropriate as in case of allowing the class initialization to fail all non-first calls will lead to `NoClassDefFoundError` which will be counterintuitive from users' perspective although the behaviour is simply unspecified for this in contract of `nameUUIDFromBytes` (Javadoc of `MessageDigest` requires the existence of `SHA-1` and `SHA-256` but nothing is said about `MD5` (should a bug-report be raised for the purpose of specifying this detail in the Javadoc?).
> 
>> Are you planning to add a microbenchmark to demonstrate the issue?
> 
> As for testing I am ready to write a benchmark but I am unsure what is the canonical way to write the one comparing the value before and after changeset. Or should it simply be a comparison of the very approach used here, not exactly the changed method (i.e. writing the benchmarks using the try/catch variant and holder variant and comparing these)? Could you please give any example of such if there are some.
> 
> Thanks in advance!

Can you look in test/micro for existing examples?

It might be that the right place to cache is in MessageDigest rather than UUID so that other code can benefit too.

One other point is that Standard Algorithms specifies that MD5 is supported by MessageDigest so the JDK would be broken it could not be found. If the eventual patch is in UUID then this aspect will need a bit of cleanup.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1821