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

[Stuart Marks]

On Fri, 18 Dec 2020 19:04:36 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>>> MD5 and DES were removed as SE requirements in JDK 14. See https://bugs.openjdk.java.net/browse/JDK-8214483 for more information. However, there are no plans to remove the implementations from the JDK at this time.
>> 
>> In this case, should a bug report be filled to require specifying behaviour for `UUID#nameUUIDFromBytes(byte[])` in case of MD5 not being present?
>
>> > MD5 and DES were removed as SE requirements in JDK 14. See https://bugs.openjdk.java.net/browse/JDK-8214483 for more information. However, there are no plans to remove the implementations from the JDK at this time.
>> 
>> In this case, should a bug report be filled to require specifying behaviour for `UUID#nameUUIDFromBytes(byte[])` in case of MD5 not being present?
> 
> Perhaps it should throw `UnsupportedOperationException`, but I'll defer to someone who works in this area. It may also be useful to support type 5 UUIDs which use SHA-1.

I have to say that introducing a ThreadLocal here seems like a step in the wrong direction. With a ThreadLocal, if I read this correctly, a MessageDigest will be cached with each thread that ever calls this API, and it won't be garbage collected until the thread dies. Some threads live indefinitely, e.g., the ones in the fork-join common pool. Is this what we want? Is UUID creation so frequent that each thread needs its own, or could thread safety be handled using a simple locking protocol?

I'd like to see a demonstration that fetching and allocating an MD5 MessageDigest instance on each call is indeed a performance problem (either because it's too slow, or because it takes too much memory). Based on that we might or might not want to do some optimization of some form or another. Or we might want to optimize creation in MessageDigest instead of caching instances in UUID. More fundamentally, I'd like to see an answer to the question, "What problem is this change intended to solve?"

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

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