8259065: Optimize MessageDigest.getInstance

Reviewed-by: valeriep

src/java.base/share/classes/java/security/MessageDigest.java

@@ -30,6 +30,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import java.nio.ByteBuffer;
 
+import sun.security.jca.GetInstance;
 import sun.security.util.Debug;
 import sun.security.util.MessageDigestSpi2;
 
@@ -176,18 +177,19 @@ public abstract class MessageDigest extends MessageDigestSpi {
      * @see Provider
      */
     public static MessageDigest getInstance(String algorithm)
-    throws NoSuchAlgorithmException {
+        throws NoSuchAlgorithmException
+    {
         Objects.requireNonNull(algorithm, "null algorithm name");
-        try {
         MessageDigest md;
-            Object[] objs = Security.getImpl(algorithm, "MessageDigest",
+
-                                             (String)null);
+        GetInstance.Instance instance = GetInstance.getInstance("MessageDigest",
-            if (objs[0] instanceof MessageDigest) {
+                MessageDigestSpi.class, algorithm);
-                md = (MessageDigest)objs[0];
+        if (instance.impl instanceof MessageDigest messageDigest) {
-                md.provider = (Provider)objs[1];
+            md = messageDigest;
+            md.provider = instance.provider;
         } else {
-                md = Delegate.of((MessageDigestSpi)objs[0], algorithm,
+            md = Delegate.of((MessageDigestSpi)instance.impl, algorithm,
-                    (Provider) objs[1]);
+                instance.provider);
         }
 
         if (!skipDebug && pdebug != null) {
@@ -196,10 +198,6 @@ public abstract class MessageDigest extends MessageDigestSpi {
         }
 
         return md;
-
-        } catch(NoSuchProviderException e) {
-            throw new NoSuchAlgorithmException(algorithm + " not found");
-        }
     }
 
     /**
@@ -245,17 +243,18 @@ public abstract class MessageDigest extends MessageDigestSpi {
         Objects.requireNonNull(algorithm, "null algorithm name");
         if (provider == null || provider.isEmpty())
             throw new IllegalArgumentException("missing provider");
-        Object[] objs = Security.getImpl(algorithm, "MessageDigest", provider);
+
-        if (objs[0] instanceof MessageDigest) {
+        MessageDigest md;
-            MessageDigest md = (MessageDigest)objs[0];
+        GetInstance.Instance instance = GetInstance.getInstance("MessageDigest",
-            md.provider = (Provider)objs[1];
+                MessageDigestSpi.class, algorithm, provider);
-            return md;
+        if (instance.impl instanceof MessageDigest messageDigest) {
+            md = messageDigest;
+            md.provider = instance.provider;
         } else {
-            MessageDigest delegate =
+            md = Delegate.of((MessageDigestSpi)instance.impl, algorithm,
-                    Delegate.of((MessageDigestSpi)objs[0], algorithm,
+                    instance.provider);
-                    (Provider)objs[1]);
-            return delegate;
         }
+        return md;
     }
 
     /**

src/java.base/share/classes/java/security/Provider.java

@@ -147,44 +147,7 @@ public abstract class Provider extends Properties {
 
     private transient boolean initialized;
 
-    private static Object newInstanceUtil(final Class<?> clazz,
+    private static final Object[] EMPTY = new Object[0];
-        final Class<?> ctrParamClz, final Object ctorParamObj)
-        throws Exception {
-        if (ctrParamClz == null) {
-            Constructor<?> con = clazz.getConstructor();
-            return con.newInstance();
-        } else {
-            // Looking for the constructor with a params first and fallback
-            // to one without if not found. This is to support the enhanced
-            // SecureRandom where both styles of constructors are supported.
-            // Before jdk9, there was no params support (only getInstance(alg))
-            // and an impl only had the params-less constructor. Since jdk9,
-            // there is getInstance(alg,params) and an impl can contain
-            // an Impl(params) constructor.
-            try {
-                Constructor<?> con = clazz.getConstructor(ctrParamClz);
-                return con.newInstance(ctorParamObj);
-            } catch (NoSuchMethodException nsme) {
-                // For pre-jdk9 SecureRandom implementations, they only
-                // have params-less constructors which still works when
-                // the input ctorParamObj is null.
-                //
-                // For other primitives using params, ctorParamObj should not
-                // be null and nsme is thrown, just like before.
-                if (ctorParamObj == null) {
-                    try {
-                        Constructor<?> con = clazz.getConstructor();
-                        return con.newInstance();
-                    } catch (NoSuchMethodException nsme2) {
-                        nsme.addSuppressed(nsme2);
-                        throw nsme;
-                    }
-                } else {
-                    throw nsme;
-                }
-            }
-        }
-    }
 
     private static double parseVersionStr(String s) {
         try {
@@ -1106,16 +1069,15 @@ public abstract class Provider extends Properties {
             this.algorithm = intern ? algorithm.intern() : algorithm;
         }
         public int hashCode() {
-            return Objects.hash(type, algorithm);
+            return type.hashCode() * 31 + algorithm.hashCode();
         }
         public boolean equals(Object obj) {
             if (this == obj) {
                 return true;
             }
-            if (!(obj instanceof ServiceKey)) {
+            if (!(obj instanceof ServiceKey other)) {
                 return false;
             }
-            ServiceKey other = (ServiceKey)obj;
             return this.type.equals(other.type)
                 && this.algorithm.equals(other.algorithm);
         }
@@ -1192,9 +1154,7 @@ public abstract class Provider extends Properties {
             ServiceKey key = new ServiceKey(type, stdAlg, true);
             Service s = legacyMap.get(key);
             if (s == null) {
-                s = new Service(this);
+                s = new Service(this, type, stdAlg);
-                s.type = type;
-                s.algorithm = stdAlg;
                 legacyMap.put(key, s);
             }
             legacyMap.put(new ServiceKey(type, aliasAlg, true), s);
@@ -1213,9 +1173,7 @@ public abstract class Provider extends Properties {
                 ServiceKey key = new ServiceKey(type, stdAlg, true);
                 Service s = legacyMap.get(key);
                 if (s == null) {
-                    s = new Service(this);
+                    s = new Service(this, type, stdAlg);
-                    s.type = type;
-                    s.algorithm = stdAlg;
                     legacyMap.put(key, s);
                 }
                 s.className = className;
@@ -1238,9 +1196,7 @@ public abstract class Provider extends Properties {
                 ServiceKey key = new ServiceKey(type, stdAlg, true);
                 Service s = legacyMap.get(key);
                 if (s == null) {
-                    s = new Service(this);
+                    s = new Service(this, type, stdAlg);
-                    s.type = type;
-                    s.algorithm = stdAlg;
                     legacyMap.put(key, s);
                 }
                 s.addAttribute(attributeName, attributeValue);
@@ -1673,14 +1629,24 @@ public abstract class Provider extends Properties {
      * @since 1.5
      */
     public static class Service {
-
+        private final String type;
-        private String type, algorithm, className;
+        private final String algorithm;
+        private String className;
         private final Provider provider;
         private List<String> aliases;
         private Map<UString,String> attributes;
+        private final EngineDescription engineDescription;
 
-        // Reference to the cached implementation Class object
+        // Reference to the cached implementation Class object.
-        private volatile Reference<Class<?>> classRef;
+        // Will be a Class if this service is loaded from the built-in
+        // classloader (unloading not possible), otherwise a WeakReference to a
+        // Class
+        private Object classCache;
+
+        // Will be a Constructor if this service is loaded from the built-in
+        // classloader (unloading not possible), otherwise a WeakReference to
+        // a Constructor
+        private Object constructorCache;
 
         // flag indicating whether this service has its attributes for
         // supportedKeyFormats or supportedKeyClasses set
@@ -1702,8 +1668,11 @@ public abstract class Provider extends Properties {
         // this constructor and these methods are used for parsing
         // the legacy string properties.
 
-        private Service(Provider provider) {
+        private Service(Provider provider, String type, String algorithm) {
             this.provider = provider;
+            this.type = type;
+            this.algorithm = algorithm;
+            engineDescription = knownEngines.get(type);
             aliases = Collections.<String>emptyList();
             attributes = Collections.<UString,String>emptyMap();
         }
@@ -1749,6 +1718,7 @@ public abstract class Provider extends Properties {
             }
             this.provider = provider;
             this.type = getEngineName(type);
+            engineDescription = knownEngines.get(type);
             this.algorithm = algorithm;
             this.className = className;
             if (aliases == null) {
@@ -1863,7 +1833,7 @@ public abstract class Provider extends Properties {
             }
             Class<?> ctrParamClz;
             try {
-                EngineDescription cap = knownEngines.get(type);
+                EngineDescription cap = engineDescription;
                 if (cap == null) {
                     // unknown engine type, use generic code
                     // this is the code path future for non-core
@@ -1890,7 +1860,7 @@ public abstract class Provider extends Properties {
                     }
                 }
                 // constructorParameter can be null if not provided
-                return newInstanceUtil(getImplClass(), ctrParamClz, constructorParameter);
+                return newInstanceUtil(ctrParamClz, constructorParameter);
             } catch (NoSuchAlgorithmException e) {
                 throw e;
             } catch (InvocationTargetException e) {
@@ -1906,11 +1876,59 @@ public abstract class Provider extends Properties {
             }
         }
 
+        private Object newInstanceOf() throws Exception {
+            Constructor<?> con = getDefaultConstructor();
+            return con.newInstance(EMPTY);
+        }
+
+        private Object newInstanceUtil(Class<?> ctrParamClz, Object ctorParamObj)
+                throws Exception
+        {
+            if (ctrParamClz == null) {
+                return newInstanceOf();
+            } else {
+                // Looking for the constructor with a params first and fallback
+                // to one without if not found. This is to support the enhanced
+                // SecureRandom where both styles of constructors are supported.
+                // Before jdk9, there was no params support (only getInstance(alg))
+                // and an impl only had the params-less constructor. Since jdk9,
+                // there is getInstance(alg,params) and an impl can contain
+                // an Impl(params) constructor.
+                try {
+                    Constructor<?> con = getImplClass().getConstructor(ctrParamClz);
+                    return con.newInstance(ctorParamObj);
+                } catch (NoSuchMethodException nsme) {
+                    // For pre-jdk9 SecureRandom implementations, they only
+                    // have params-less constructors which still works when
+                    // the input ctorParamObj is null.
+                    //
+                    // For other primitives using params, ctorParamObj should not
+                    // be null and nsme is thrown, just like before.
+                    if (ctorParamObj == null) {
+                        try {
+                            return newInstanceOf();
+                        } catch (NoSuchMethodException nsme2) {
+                            nsme.addSuppressed(nsme2);
+                            throw nsme;
+                        }
+                    } else {
+                        throw nsme;
+                    }
+                }
+            }
+        }
+
         // return the implementation Class object for this service
         private Class<?> getImplClass() throws NoSuchAlgorithmException {
             try {
-                Reference<Class<?>> ref = classRef;
+                Object cache = classCache;
-                Class<?> clazz = (ref == null) ? null : ref.get();
+                if (cache instanceof Class<?> clazz) {
+                    return clazz;
+                }
+                Class<?> clazz = null;
+                if (cache instanceof WeakReference<?> ref){
+                    clazz = (Class<?>)ref.get();
+                }
                 if (clazz == null) {
                     ClassLoader cl = provider.getClass().getClassLoader();
                     if (cl == null) {
@@ -1923,7 +1941,7 @@ public abstract class Provider extends Properties {
                             ("class configured for " + type + " (provider: " +
                             provider.getName() + ") is not public.");
                     }
-                    classRef = new WeakReference<>(clazz);
+                    classCache = (cl == null) ? clazz : new WeakReference<Class<?>>(clazz);
                 }
                 return clazz;
             } catch (ClassNotFoundException e) {
@@ -1933,6 +1951,26 @@ public abstract class Provider extends Properties {
             }
         }
 
+        private Constructor<?> getDefaultConstructor()
+            throws NoSuchAlgorithmException, NoSuchMethodException
+        {
+            Object cache = constructorCache;
+            if (cache instanceof Constructor<?> con) {
+                return con;
+            }
+            Constructor<?> con = null;
+            if (cache instanceof WeakReference<?> ref){
+                con = (Constructor<?>)ref.get();
+            }
+            if (con == null) {
+                Class<?> clazz = getImplClass();
+                con = clazz.getConstructor();
+                constructorCache = (clazz.getClassLoader() == null)
+                        ? con : new WeakReference<Constructor<?>>(con);
+            }
+            return con;
+        }
+
         /**
          * Test whether this Service can use the specified parameter.
          * Returns false if this service cannot use the parameter. Returns
@@ -1960,7 +1998,7 @@ public abstract class Provider extends Properties {
          * used with this type of service
          */
         public boolean supportsParameter(Object parameter) {
-            EngineDescription cap = knownEngines.get(type);
+            EngineDescription cap = engineDescription;
             if (cap == null) {
                 // unknown engine type, return true by default
                 return true;

src/java.base/share/classes/sun/security/jca/ProviderConfig.java

@@ -160,12 +160,18 @@ final class ProviderConfig {
      * Get the provider object. Loads the provider if it is not already loaded.
      */
     @SuppressWarnings("deprecation")
-    synchronized Provider getProvider() {
+    Provider getProvider() {
         // volatile variable load
         Provider p = provider;
         if (p != null) {
             return p;
         }
+        // DCL
+        synchronized (this) {
+            p = provider;
+            if (p != null) {
+                return p;
+            }
             if (shouldLoad() == false) {
                 return null;
             }
@@ -220,6 +226,7 @@ final class ProviderConfig {
                 }
             }
             provider = p;
+        }
         return p;
     }
 

test/micro/org/openjdk/bench/java/security/GetMessageDigest.java

@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2021, 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;
+
+/**
+ * Micros for speed of looking up and instantiating 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 {
+
+    @Param({"md5", "SHA-1", "SHA-256"})
+    private String digesterName;
+
+    private MessageDigest messageDigest;
+
+    @Setup
+    public void setupMessageDigestForCloning() throws NoSuchAlgorithmException {
+        messageDigest = MessageDigest.getInstance(digesterName);
+    }
+
+    @Benchmark
+    public MessageDigest getInstance() throws NoSuchAlgorithmException {
+        return MessageDigest.getInstance(digesterName);
+    }
+
+    @Benchmark
+    public MessageDigest cloneInstance() throws NoSuchAlgorithmException, CloneNotSupportedException {
+        return (MessageDigest)messageDigest.clone();
+    }
+
+    @Benchmark
+    public MessageDigest getInstanceWithProvider() throws NoSuchAlgorithmException, NoSuchProviderException {
+        return MessageDigest.getInstance(digesterName, "SUN");
+    }
+}