changeset 59301:fe95add5e0fc

8245024: Simplify and eagerly initialize StringConcatFactory Reviewed-by: psandoz
author redestad
date Fri, 15 May 2020 12:25:37 +0200
parents b3a871b1dc3f
children fa5a0fb28d3d
files src/java.base/share/classes/java/lang/System.java src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java
diffstat 2 files changed, 88 insertions(+), 93 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/lang/System.java	Fri May 15 12:09:59 2020 +0200
+++ b/src/java.base/share/classes/java/lang/System.java	Fri May 15 12:25:37 2020 +0200
@@ -37,6 +37,7 @@
 import java.lang.annotation.Annotation;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodType;
+import java.lang.invoke.StringConcatFactory;
 import java.lang.module.ModuleDescriptor;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Executable;
@@ -62,6 +63,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Stream;
 
+import jdk.internal.misc.Unsafe;
 import jdk.internal.util.StaticProperty;
 import jdk.internal.module.ModuleBootstrap;
 import jdk.internal.module.ServicesCatalog;
@@ -2049,6 +2051,7 @@
      * @return JNI_OK for success, JNI_ERR for failure
      */
     private static int initPhase2(boolean printToStderr, boolean printStackTrace) {
+
         try {
             bootLayer = ModuleBootstrap.boot();
         } catch (Exception | Error e) {
@@ -2065,15 +2068,23 @@
 
     /*
      * Invoked by VM.  Phase 3 is the final system initialization:
-     * 1. set security manager
-     * 2. set system class loader
-     * 3. set TCCL
+     * 1. eagerly initialize bootstrap method factories that might interact
+     *    negatively with custom security managers and custom class loaders
+     * 2. set security manager
+     * 3. set system class loader
+     * 4. set TCCL
      *
      * This method must be called after the module system initialization.
      * The security manager and system class loader may be a custom class from
      * the application classpath or modulepath.
      */
     private static void initPhase3() {
+
+        // Initialize the StringConcatFactory eagerly to avoid potential
+        // bootstrap circularity issues that could be caused by a custom
+        // SecurityManager
+        Unsafe.getUnsafe().ensureClassInitialized(StringConcatFactory.class);
+
         String smProp = System.getProperty("java.security.manager");
         if (smProp != null) {
             switch (smProp) {
--- a/src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java	Fri May 15 12:09:59 2020 +0200
+++ b/src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java	Fri May 15 12:25:37 2020 +0200
@@ -129,13 +129,10 @@
     /**
      * Concatenation strategy to use. See {@link Strategy} for possible options.
      * This option is controllable with -Djava.lang.invoke.stringConcat JDK option.
+     *
+     * Defaults to MH_INLINE_SIZED_EXACT if not set.
      */
-    private static Strategy STRATEGY;
-
-    /**
-     * Default strategy to use for concatenation.
-     */
-    private static final Strategy DEFAULT_STRATEGY = Strategy.MH_INLINE_SIZED_EXACT;
+    private static final Strategy STRATEGY;
 
     private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess();
 
@@ -182,42 +179,13 @@
      */
     private static final boolean DEBUG;
 
-    /**
-     * Enables caching of strategy stubs. This may improve the linkage time by reusing the generated
-     * code, at the expense of contaminating the profiles.
-     */
-    private static final boolean CACHE_ENABLE;
-
-    private static final ConcurrentMap<Key, MethodHandle> CACHE;
-
-    /**
-     * Dump generated classes to disk, for debugging purposes.
-     */
-    private static final ProxyClassesDumper DUMPER;
-
     static {
-        // In case we need to double-back onto the StringConcatFactory during this
-        // static initialization, make sure we have the reasonable defaults to complete
-        // the static initialization properly. After that, actual users would use
-        // the proper values we have read from the properties.
-        STRATEGY = DEFAULT_STRATEGY;
-        // CACHE_ENABLE = false; // implied
-        // CACHE = null;         // implied
-        // DEBUG = false;        // implied
-        // DUMPER = null;        // implied
-
         final String strategy =
                 VM.getSavedProperty("java.lang.invoke.stringConcat");
-        CACHE_ENABLE = Boolean.parseBoolean(
-                VM.getSavedProperty("java.lang.invoke.stringConcat.cache"));
+        STRATEGY = (strategy == null) ? null : Strategy.valueOf(strategy);
+
         DEBUG = Boolean.parseBoolean(
                 VM.getSavedProperty("java.lang.invoke.stringConcat.debug"));
-        final String dumpPath =
-                VM.getSavedProperty("java.lang.invoke.stringConcat.dumpClasses");
-
-        STRATEGY = (strategy == null) ? DEFAULT_STRATEGY : Strategy.valueOf(strategy);
-        CACHE = CACHE_ENABLE ? new ConcurrentHashMap<>() : null;
-        DUMPER = (dumpPath == null) ? null : ProxyClassesDumper.getInstance(dumpPath);
     }
 
     /**
@@ -260,7 +228,7 @@
     }
 
     /**
-     * Parses the recipe string, and produces the traversable collection of
+     * Parses the recipe string, and produces a traversable collection of
      * {@link java.lang.invoke.StringConcatFactory.RecipeElement}-s for generator
      * strategies. Notably, this class parses out the constants from the recipe
      * and from other static arguments.
@@ -668,21 +636,9 @@
                     MAX_INDY_CONCAT_ARG_SLOTS);
         }
 
-        String className = getClassName(lookup.lookupClass());
         MethodType mt = adaptType(concatType);
         Recipe rec = new Recipe(recipe, constants);
-
-        MethodHandle mh;
-        if (CACHE_ENABLE) {
-            Key key = new Key(className, mt, rec);
-            mh = CACHE.get(key);
-            if (mh == null) {
-                mh = generate(lookup, className, mt, rec);
-                CACHE.put(key, mh);
-            }
-        } else {
-            mh = generate(lookup, className, mt, rec);
-        }
+        MethodHandle mh = generate(lookup, mt, rec);
         return new ConstantCallSite(mh.asType(concatType));
     }
 
@@ -714,46 +670,18 @@
                 : args;
     }
 
-    private static String getClassName(Class<?> hostClass) throws StringConcatException {
-        /*
-          The generated class is in the same package as the host class as
-          it's the implementation of the string concatenation for the host class.
-
-          When cache is enabled, we want to cache as much as we can.
-         */
-
-        switch (STRATEGY) {
-            case BC_SB:
-            case BC_SB_SIZED:
-            case BC_SB_SIZED_EXACT: {
-                if (CACHE_ENABLE) {
-                    String pkgName = hostClass.getPackageName();
-                    return (!pkgName.isEmpty() ? pkgName.replace('.', '/') + "/" : "") + "Stubs$$StringConcat";
-                } else {
-                    String name = hostClass.isHidden() ? hostClass.getName().replace('/', '_')
-                                                       : hostClass.getName();
-                    return name.replace('.', '/') + "$$StringConcat";
-                }
+    private static MethodHandle generate(Lookup lookup, MethodType mt, Recipe recipe) throws StringConcatException {
+        try {
+            if (STRATEGY == null) {
+                return MethodHandleInlineCopyStrategy.generate(mt, recipe);
             }
-            case MH_SB_SIZED:
-            case MH_SB_SIZED_EXACT:
-            case MH_INLINE_SIZED_EXACT:
-                // MethodHandle strategies do not need a class name.
-                return "";
-            default:
-                throw new StringConcatException("Concatenation strategy " + STRATEGY + " is not implemented");
-        }
-    }
-
-    private static MethodHandle generate(Lookup lookup, String className, MethodType mt, Recipe recipe) throws StringConcatException {
-        try {
             switch (STRATEGY) {
                 case BC_SB:
-                    return BytecodeStringBuilderStrategy.generate(lookup, className, mt, recipe, Mode.DEFAULT);
+                    return BytecodeStringBuilderStrategy.generate(lookup, mt, recipe, Mode.DEFAULT);
                 case BC_SB_SIZED:
-                    return BytecodeStringBuilderStrategy.generate(lookup, className, mt, recipe, Mode.SIZED);
+                    return BytecodeStringBuilderStrategy.generate(lookup, mt, recipe, Mode.SIZED);
                 case BC_SB_SIZED_EXACT:
-                    return BytecodeStringBuilderStrategy.generate(lookup, className, mt, recipe, Mode.SIZED_EXACT);
+                    return BytecodeStringBuilderStrategy.generate(lookup, mt, recipe, Mode.SIZED_EXACT);
                 case MH_SB_SIZED:
                     return MethodHandleStringBuilderStrategy.generate(mt, recipe, Mode.SIZED);
                 case MH_SB_SIZED_EXACT:
@@ -836,11 +764,45 @@
         static final int CLASSFILE_VERSION = 52;
         static final String METHOD_NAME = "concat";
 
+        private static final ConcurrentMap<Key, MethodHandle> CACHE;
+
+        /**
+         * Enables caching of strategy stubs. This may improve the linkage time by reusing the generated
+         * code, at the expense of contaminating the profiles.
+         */
+        private static final boolean CACHE_ENABLE;
+
+        /**
+         * Dump generated classes to disk, for debugging purposes.
+         */
+        private static final ProxyClassesDumper DUMPER;
+
+        static {
+            CACHE_ENABLE = Boolean.parseBoolean(
+                    VM.getSavedProperty("java.lang.invoke.stringConcat.cache"));
+            CACHE = CACHE_ENABLE ? new ConcurrentHashMap<>() : null;
+
+            final String dumpPath =
+                    VM.getSavedProperty("java.lang.invoke.stringConcat.dumpClasses");
+
+            DUMPER = (dumpPath == null) ? null : ProxyClassesDumper.getInstance(dumpPath);
+        }
+
         private BytecodeStringBuilderStrategy() {
             // no instantiation
         }
 
-        private static MethodHandle generate(Lookup lookup, String className, MethodType args, Recipe recipe, Mode mode) throws Exception {
+        private static MethodHandle generate(Lookup lookup, MethodType args, Recipe recipe, Mode mode) throws Exception {
+            String className = getClassName(lookup.lookupClass());
+            Key key = null;
+            if (CACHE_ENABLE) {
+                key = new Key(className, args, recipe);
+                MethodHandle mh = CACHE.get(key);
+                if (mh != null) {
+                    return mh;
+                }
+            }
+
             ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS + ClassWriter.COMPUTE_FRAMES);
 
             cw.visit(CLASSFILE_VERSION,
@@ -1130,13 +1092,35 @@
             try {
                 Class<?> innerClass = lookup.defineHiddenClass(classBytes, true, STRONG).lookupClass();
                 dumpIfEnabled(className, classBytes);
-                return lookup.findStatic(innerClass, METHOD_NAME, args);
+                MethodHandle mh = lookup.findStatic(innerClass, METHOD_NAME, args);
+                if (CACHE_ENABLE) {
+                    CACHE.put(key, mh);
+                }
+                return mh;
             } catch (Exception e) {
                 dumpIfEnabled(className + "$$FAILED", classBytes);
                 throw new StringConcatException("Exception while spinning the class", e);
             }
         }
 
+        /**
+         * The generated class is in the same package as the host class as
+         * it's the implementation of the string concatenation for the host
+         * class.
+         *
+         * When cache is enabled, we want to cache as much as we can.
+         */
+        private static String getClassName(Class<?> hostClass) {
+            if (CACHE_ENABLE) {
+                String pkgName = hostClass.getPackageName();
+                return (!pkgName.isEmpty() ? pkgName.replace('.', '/') + "/" : "") + "Stubs$$StringConcat";
+            } else {
+                String name = hostClass.isHidden() ? hostClass.getName().replace('/', '_')
+                        : hostClass.getName();
+                return name.replace('.', '/') + "$$StringConcat";
+            }
+        }
+
         private static void dumpIfEnabled(String name, byte[] bytes) {
             if (DUMPER != null) {
                 DUMPER.dumpClass(name, bytes);
@@ -1707,7 +1691,7 @@
         private static MethodHandle prepender(String prefix, Class<?> cl, String suffix) {
             return MethodHandles.insertArguments(
                     MethodHandles.insertArguments(
-                        PREPENDERS.computeIfAbsent(cl, PREPEND),2, prefix), 3, suffix);
+                        PREPENDERS.computeIfAbsent(cl, PREPEND), 2, prefix), 3, suffix);
         }
 
         private static MethodHandle mixer(Class<?> cl) {
@@ -1752,7 +1736,7 @@
 
             SIMPLE     = JLA.stringConcatHelper("simpleConcat", methodType(String.class, Object.class, Object.class));
             NEW_STRING = JLA.stringConcatHelper("newString", methodType(String.class, byte[].class, long.class));
-            NEW_ARRAY  = JLA.stringConcatHelper( "newArray", methodType(byte[].class, long.class));
+            NEW_ARRAY  = JLA.stringConcatHelper("newArray", methodType(byte[].class, long.class));
         }
     }