8171400: Move checking of duplicate packages in the boot layer to link time
authorredestad
Mon, 19 Dec 2016 21:38:46 +0100
changeset 42770 d363e7c3584a
parent 42769 c6ee006baceb
child 42771 8c4a539300af
8171400: Move checking of duplicate packages in the boot layer to link time Reviewed-by: alanb
jdk/src/java.base/share/classes/java/lang/reflect/Layer.java
jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
jdk/src/java.base/share/classes/jdk/internal/module/SystemModules.java
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
--- a/jdk/src/java.base/share/classes/java/lang/reflect/Layer.java	Mon Dec 19 09:56:11 2016 -0800
+++ b/jdk/src/java.base/share/classes/java/lang/reflect/Layer.java	Mon Dec 19 21:38:46 2016 +0100
@@ -602,12 +602,8 @@
 
         checkGetClassLoaderPermission();
 
-        // For now, no two modules in the boot Layer may contain the same
-        // package so we use a simple check for the boot Layer to keep
-        // the overhead at startup to a minimum
-        if (boot() == null) {
-            checkBootModulesForDuplicatePkgs(cf);
-        } else {
+        // The boot layer is checked during module system initialization
+        if (boot() != null) {
             checkForDuplicatePkgs(cf, clf);
         }
 
@@ -657,27 +653,6 @@
     }
 
     /**
-     * Checks a configuration for the boot Layer to ensure that no two modules
-     * have the same package.
-     *
-     * @throws LayerInstantiationException
-     */
-    private static void checkBootModulesForDuplicatePkgs(Configuration cf) {
-        Map<String, String> packageToModule = new HashMap<>();
-        for (ResolvedModule resolvedModule : cf.modules()) {
-            ModuleDescriptor descriptor = resolvedModule.reference().descriptor();
-            String name = descriptor.name();
-            for (String p : descriptor.packages()) {
-                String other = packageToModule.putIfAbsent(p, name);
-                if (other != null) {
-                    throw fail("Package " + p + " in both module "
-                               + name + " and module " + other);
-                }
-            }
-        }
-    }
-
-    /**
      * Checks a configuration and the module-to-loader mapping to ensure that
      * no two modules mapped to the same class loader have the same package.
      * It also checks that no two automatic modules have the same package.
--- a/jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java	Mon Dec 19 09:56:11 2016 -0800
+++ b/jdk/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java	Mon Dec 19 21:38:46 2016 +0100
@@ -308,6 +308,23 @@
             }
         }
 
+        // if needed check that there are no split packages in the set of
+        // resolved modules for the boot layer
+        if (SystemModules.hasSplitPackages() || needPostResolutionChecks) {
+                Map<String, String> packageToModule = new HashMap<>();
+                for (ResolvedModule resolvedModule : cf.modules()) {
+                    ModuleDescriptor descriptor =
+                        resolvedModule.reference().descriptor();
+                    String name = descriptor.name();
+                    for (String p : descriptor.packages()) {
+                        String other = packageToModule.putIfAbsent(p, name);
+                        if (other != null) {
+                            fail("Package " + p + " in both module "
+                                 + name + " and module " + other);
+                        }
+                    }
+                }
+            }
 
         long t4 = System.nanoTime();
 
--- a/jdk/src/java.base/share/classes/jdk/internal/module/SystemModules.java	Mon Dec 19 09:56:11 2016 -0800
+++ b/jdk/src/java.base/share/classes/jdk/internal/module/SystemModules.java	Mon Dec 19 21:38:46 2016 +0100
@@ -57,6 +57,14 @@
     public static int PACKAGES_IN_BOOT_LAYER = 1024;
 
     /**
+     * @return {@code false} if there are no split packages in the run-time
+     *         image, {@code true} if there are or if it's not been checked.
+     */
+    public static boolean hasSplitPackages() {
+        return true;
+    }
+
+    /**
      * Returns a non-empty array of ModuleDescriptors in the run-time image.
      *
      * When running an exploded image it returns an empty array.
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java	Mon Dec 19 09:56:11 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java	Mon Dec 19 21:38:46 2016 +0100
@@ -34,6 +34,7 @@
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -342,7 +343,8 @@
          *
          * static Map<String, ModuleDescriptor> map = new HashMap<>();
          */
-        private void clinit(int numModules, int numPackages) {
+        private void clinit(int numModules, int numPackages,
+                            boolean hasSplitPackages) {
             cw.visit(Opcodes.V1_8, ACC_PUBLIC+ACC_FINAL+ACC_SUPER, CLASSNAME,
                      null, "java/lang/Object", null);
 
@@ -379,6 +381,17 @@
             clinit.visitInsn(RETURN);
             clinit.visitMaxs(0, 0);
             clinit.visitEnd();
+
+            // public static boolean hasSplitPackages();
+            MethodVisitor split =
+                cw.visitMethod(ACC_PUBLIC+ACC_STATIC, "hasSplitPackages",
+                               "()Z", null, null);
+            split.visitCode();
+            split.visitInsn(hasSplitPackages ? ICONST_1 : ICONST_0);
+            split.visitInsn(IRETURN);
+            split.visitMaxs(0, 0);
+            split.visitEnd();
+
         }
 
         /*
@@ -416,12 +429,16 @@
          */
         public ClassWriter getClassWriter() {
             int numModules = moduleInfos.size();
-            int numPackages = 0;
+            Set<String> allPackages = new HashSet<>();
+            int packageCount = 0;
             for (ModuleInfo minfo : moduleInfos) {
-                numPackages += minfo.packages.size();
+                allPackages.addAll(minfo.packages);
+                packageCount += minfo.packages.size();
             }
 
-            clinit(numModules, numPackages);
+            int numPackages = allPackages.size();
+            boolean hasSplitPackages = (numPackages < packageCount);
+            clinit(numModules, numPackages, hasSplitPackages);
 
             // generate SystemModules::descriptors
             genDescriptorsMethod();