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

[Claes Redestad]

On Fri, 18 Dec 2020 13:39:48 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>>> 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.

There are pre-existing microbenchmarks for java.security under test/micro/org/openjdk/bench/java/security

You can build and run these using `make test TEST=micro:YourBenchmark`. Refer to [doc/testing.md](https://github.com/openjdk/jdk/blob/master/doc/testing.md) for some required configuration steps. 

I'd suggest starting with a simple micro that zooms in on MessageDigest.getInstance("MD5"), maybe like so:

/*
 * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2 only, as
 * published by the Free Software Foundation.
 *
 * This code is distributed in the hope that it will be useful, but WITHOUT
 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
 * version 2 for more details (a copy is included in the LICENSE file that
 * accompanied this code).
 *
 * You should have received a copy of the GNU General Public License version
 * 2 along with this work; if not, write to the Free Software Foundation,
 * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
 *
 * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
 * or visit www.oracle.com if you need additional information or have any
 * questions.
 */
package org.openjdk.bench.java.security;

import java.security.DigestException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.util.Random;
import java.util.concurrent.TimeUnit;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;

/**
 * Tests speed of looking up MessageDigests.
 */
@State(Scope.Thread)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 5, time = 1)
@Measurement(iterations = 10, time = 1)
@Fork(value = 3)
public class GetMessageDigest {

    @Benchmark
    @Warmup(iterations = 5, time = 1)
    @Measurement(iterations = 10, time = 1)
    public MessageDigest getMD5Digest() throws NoSuchAlgorithmException {
        return MessageDigest.getInstance("MD5");
    }
}

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

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