From ec7da91bd83803b7d91a4de3a01caf0ba256c037 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 6 Jul 2023 16:08:36 +0000 Subject: [PATCH] 8240567: MethodTooLargeException thrown while creating a jlink image Reviewed-by: mchung --- .../internal/plugins/SystemModulesPlugin.java | 163 ++++++++++++++++-- .../tools/jlink/resources/plugins.properties | 13 +- test/jdk/tools/jlink/JLink100Modules.java | 128 ++++++++++++++ 3 files changed, 285 insertions(+), 19 deletions(-) create mode 100644 test/jdk/tools/jlink/JLink100Modules.java diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java index c2dd257c57a..77ba8c063e8 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java @@ -120,6 +120,8 @@ public final class SystemModulesPlugin extends AbstractPlugin { ClassDesc.ofInternalName(SYSTEM_MODULES_MAP_CLASSNAME); private static final MethodTypeDesc MTD_StringArray = MethodTypeDesc.of(CD_String.arrayType()); private static final MethodTypeDesc MTD_SystemModules = MethodTypeDesc.of(CD_SYSTEM_MODULES); + + private int moduleDescriptorsPerMethod = 75; private boolean enabled; public SystemModulesPlugin() { @@ -142,7 +144,14 @@ public final class SystemModulesPlugin extends AbstractPlugin { public void configure(Map config) { String arg = config.get(getName()); if (arg != null) { - throw new IllegalArgumentException(getName() + ": " + arg); + String[] split = arg.split("="); + if (split.length != 2) { + throw new IllegalArgumentException(getName() + ": " + arg); + } + if (split[0].equals("batch-size")) { + throw new IllegalArgumentException(getName() + ": " + arg); + } + this.moduleDescriptorsPerMethod = Integer.parseInt(split[1]); } } @@ -318,7 +327,7 @@ public final class SystemModulesPlugin extends AbstractPlugin { String className, ResourcePoolBuilder out) { SystemModulesClassGenerator generator - = new SystemModulesClassGenerator(className, moduleInfos); + = new SystemModulesClassGenerator(className, moduleInfos, moduleDescriptorsPerMethod); byte[] bytes = generator.genClassBytes(cf); String rn = "/java.base/" + className + ".class"; ResourcePoolEntry e = ResourcePoolEntry.create(rn, bytes); @@ -533,11 +542,12 @@ public final class SystemModulesPlugin extends AbstractPlugin { private static final int MAX_LOCAL_VARS = 256; - private final int BUILDER_VAR = 0; private final int MD_VAR = 1; // variable for ModuleDescriptor private final int MT_VAR = 1; // variable for ModuleTarget private final int MH_VAR = 1; // variable for ModuleHashes - private int nextLocalVar = 2; // index to next local variable + private final int DEDUP_LIST_VAR = 2; + private final int BUILDER_VAR = 3; + private int nextLocalVar = 4; // index to next local variable // name of class to generate private final ClassDesc classDesc; @@ -545,6 +555,8 @@ public final class SystemModulesPlugin extends AbstractPlugin { // list of all ModuleInfos private final List moduleInfos; + private final int moduleDescriptorsPerMethod; + // A builder to create one single Set instance for a given set of // names or modifiers to reduce the footprint // e.g. target modules of qualified exports @@ -552,9 +564,11 @@ public final class SystemModulesPlugin extends AbstractPlugin { = new DedupSetBuilder(this::getNextLocalVar); public SystemModulesClassGenerator(String className, - List moduleInfos) { + List moduleInfos, + int moduleDescriptorsPerMethod) { this.classDesc = ClassDesc.ofInternalName(className); this.moduleInfos = moduleInfos; + this.moduleDescriptorsPerMethod = moduleDescriptorsPerMethod; moduleInfos.forEach(mi -> dedups(mi.descriptor())); } @@ -680,6 +694,71 @@ public final class SystemModulesPlugin extends AbstractPlugin { * Generate bytecode for moduleDescriptors method */ private void genModuleDescriptorsMethod(ClassBuilder clb) { + if (moduleInfos.size() <= moduleDescriptorsPerMethod) { + clb.withMethodBody( + "moduleDescriptors", + MTD_ModuleDescriptorArray, + ACC_PUBLIC, + cob -> { + cob.constantInstruction(moduleInfos.size()) + .anewarray(CD_MODULE_DESCRIPTOR) + .astore(MD_VAR); + + for (int index = 0; index < moduleInfos.size(); index++) { + ModuleInfo minfo = moduleInfos.get(index); + new ModuleDescriptorBuilder(cob, + minfo.descriptor(), + minfo.packages(), + index).build(); + } + cob.aload(MD_VAR) + .areturn(); + }); + return; + } + + + // Split the module descriptors be created by multiple helper methods. + // Each helper method "subi" creates the maximum N number of module descriptors + // mi, m{i+1} ... + // to avoid exceeding the 64kb limit of method length. Then it will call + // "sub{i+1}" to creates the next batch of module descriptors m{i+n}, m{i+n+1}... + // and so on. During the construction of the module descriptors, the string sets and + // modifier sets are deduplicated (see SystemModulesClassGenerator.DedupSetBuilder) + // and cached in the locals. These locals are saved in an array list so + // that the helper method can restore the local variables that may be + // referenced by the bytecode generated for creating module descriptors. + // Pseudo code looks like this: + // + // void subi(ModuleDescriptor[] mdescs, ArrayList localvars) { + // // assign localvars to local variables + // var l3 = localvars.get(0); + // var l4 = localvars.get(1); + // : + // // fill mdescs[i] to mdescs[i+n-1] + // mdescs[i] = ... + // mdescs[i+1] = ... + // : + // // save new local variables added + // localvars.add(lx) + // localvars.add(l{x+1}) + // : + // sub{i+i}(mdescs, localvars); + // } + + List> splitModuleInfos = new ArrayList<>(); + List currentModuleInfos = null; + for (int index = 0; index < moduleInfos.size(); index++) { + if (index % moduleDescriptorsPerMethod == 0) { + currentModuleInfos = new ArrayList<>(); + splitModuleInfos.add(currentModuleInfos); + } + currentModuleInfos.add(moduleInfos.get(index)); + } + + String helperMethodNamePrefix = "sub"; + ClassDesc arrayListClassDesc = ClassDesc.ofInternalName("java/util/ArrayList"); + clb.withMethodBody( "moduleDescriptors", MTD_ModuleDescriptorArray, @@ -687,18 +766,74 @@ public final class SystemModulesPlugin extends AbstractPlugin { cob -> { cob.constantInstruction(moduleInfos.size()) .anewarray(CD_MODULE_DESCRIPTOR) + .dup() .astore(MD_VAR); - - for (int index = 0; index < moduleInfos.size(); index++) { - ModuleInfo minfo = moduleInfos.get(index); - new ModuleDescriptorBuilder(cob, - minfo.descriptor(), - minfo.packages(), - index).build(); - } - cob.aload(MD_VAR) + cob.new_(arrayListClassDesc) + .dup() + .constantInstruction(moduleInfos.size()) + .invokespecial(arrayListClassDesc, INIT_NAME, MethodTypeDesc.of(CD_void, CD_int)) + .astore(DEDUP_LIST_VAR); + cob.aload(0) + .aload(MD_VAR) + .aload(DEDUP_LIST_VAR) + .invokevirtual( + this.classDesc, + helperMethodNamePrefix + "0", + MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType(), arrayListClassDesc) + ) .areturn(); }); + + int dedupVarStart = nextLocalVar; + for (int n = 0, count = 0; n < splitModuleInfos.size(); count += splitModuleInfos.get(n).size(), n++) { + int index = n; // the index of which ModuleInfo being processed in the current batch + int start = count; // the start index to the return ModuleDescriptor array for the current batch + int curDedupVar = nextLocalVar; + clb.withMethodBody( + helperMethodNamePrefix + index, + MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType(), arrayListClassDesc), + ACC_PUBLIC, + cob -> { + if (curDedupVar > dedupVarStart) { + for (int i = dedupVarStart; i < curDedupVar; i++) { + cob.aload(DEDUP_LIST_VAR) + .constantInstruction(i - dedupVarStart) + .invokevirtual(arrayListClassDesc, "get", MethodTypeDesc.of(CD_Object, CD_int)) + .astore(i); + } + } + + List currentBatch = splitModuleInfos.get(index); + for (int j = 0; j < currentBatch.size(); j++) { + ModuleInfo minfo = currentBatch.get(j); + new ModuleDescriptorBuilder(cob, + minfo.descriptor(), + minfo.packages(), + start + j).build(); + } + + if (index < splitModuleInfos.size() - 1) { + if (nextLocalVar > curDedupVar) { + for (int i = curDedupVar; i < nextLocalVar; i++) { + cob.aload(DEDUP_LIST_VAR) + .aload(i) + .invokevirtual(arrayListClassDesc, "add", MethodTypeDesc.of(CD_boolean, CD_Object)) + .pop(); + } + } + cob.aload(0) + .aload(MD_VAR) + .aload(DEDUP_LIST_VAR) + .invokevirtual( + this.classDesc, + helperMethodNamePrefix + (index+1), + MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType(), arrayListClassDesc) + ); + } + + cob.return_(); + }); + } } /** diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties index e5e8a7e7f4f..a4b780a15c3 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties @@ -147,13 +147,16 @@ generate-jli-classes.usage=\ \ correctness add ignore-version=true\n\ \ to override this. -system-modules.argument=retainModuleTarget - -system-modules.description=Fast loading of module descriptors (always enabled) +system-modules.argument=batch-size= sets the batch size of module descriptors\n\ +\ to avoid exceeding the method length limit. The default\n\ +\ batch size is 75. system-modules.usage=\ -\ --system-modules retainModuleTarget\n\ -\ Fast loading of module descriptors (always enabled) +\ --system-modules [batch-size=]\n\ +\ The batch size specifies the maximum number of modules\n\ +\ be handled in one method to workaround if the generated\n\ +\ bytecode exceeds the method size limit. The default\n\ +\ batch size is 75. onoff.argument= diff --git a/test/jdk/tools/jlink/JLink100Modules.java b/test/jdk/tools/jlink/JLink100Modules.java new file mode 100644 index 00000000000..926a6adaf4f --- /dev/null +++ b/test/jdk/tools/jlink/JLink100Modules.java @@ -0,0 +1,128 @@ +/* + * Copyright (c) 2022, 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. + */ + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.StringJoiner; +import java.util.spi.ToolProvider; + +import tests.JImageGenerator; + +/* + * @test + * @summary Make sure that 100 modules can be linked using jlink. + * @bug 8240567 + * @library ../lib + * @modules java.base/jdk.internal.jimage + * jdk.jdeps/com.sun.tools.classfile + * jdk.jlink/jdk.tools.jlink.internal + * jdk.jlink/jdk.tools.jlink.plugin + * jdk.jlink/jdk.tools.jmod + * jdk.jlink/jdk.tools.jimage + * jdk.compiler + * @build tests.* + * @run main/othervm -Xmx1g -Xlog:init=debug -XX:+UnlockDiagnosticVMOptions -XX:+BytecodeVerificationLocal JLink100Modules + */ +public class JLink100Modules { + private static final ToolProvider JAVAC_TOOL = ToolProvider.findFirst("javac") + .orElseThrow(() -> new RuntimeException("javac tool not found")); + + static void report(String command, String[] args) { + System.out.println(command + " " + String.join(" ", Arrays.asList(args))); + } + + static void javac(String[] args) { + report("javac", args); + JAVAC_TOOL.run(System.out, System.err, args); + } + + public static void main(String[] args) throws Exception { + Path src = Paths.get("bug8240567"); + + StringJoiner mainModuleInfoContent = new StringJoiner(";\n requires ", "module bug8240567x {\n requires ", ";\n}"); + + for (int i = 0; i < 1_000; i++) { + String name = "module" + i + "x"; + Path moduleDir = Files.createDirectories(src.resolve(name)); + + StringBuilder moduleInfoContent = new StringBuilder("module "); + moduleInfoContent.append(name).append(" {\n"); + if (i != 0) { + moduleInfoContent.append(" requires module0x;\n"); + } + moduleInfoContent.append("}\n"); + Files.writeString(moduleDir.resolve("module-info.java"), moduleInfoContent.toString()); + + mainModuleInfoContent.add(name); + } + + // create module reading the generated modules + Path mainModulePath = src.resolve("bug8240567x"); + Files.createDirectories(mainModulePath); + Path mainModuleInfo = mainModulePath.resolve("module-info.java"); + Files.writeString(mainModuleInfo, mainModuleInfoContent.toString()); + + Path mainClassDir = mainModulePath.resolve("testpackage"); + Files.createDirectories(mainClassDir); + + Files.writeString(mainClassDir.resolve("JLink100ModulesTest.java"), """ + package testpackage; + + public class JLink100ModulesTest { + public static void main(String[] args) throws Exception { + System.out.println("JLink100ModulesTest started."); + } + } + """); + + String out = src.resolve("out").toString(); + javac(new String[]{ + "-d", out, + "--module-source-path", src.toString(), + "--module", "bug8240567x" + }); + + JImageGenerator.getJLinkTask() + .modulePath(out) + .output(src.resolve("out-jlink")) + .addMods("bug8240567x") + .call() + .assertSuccess(); + + Path binDir = src.resolve("out-jlink").resolve("bin").toAbsolutePath(); + Path bin = binDir.resolve("java"); + + ProcessBuilder processBuilder = new ProcessBuilder(bin.toString(), + "-XX:+UnlockDiagnosticVMOptions", + "-XX:+BytecodeVerificationLocal", + "-m", "bug8240567x/testpackage.JLink100ModulesTest"); + processBuilder.inheritIO(); + processBuilder.directory(binDir.toFile()); + Process process = processBuilder.start(); + int exitCode = process.waitFor(); + if (exitCode != 0) + throw new AssertionError("JLink100ModulesTest failed to launch"); + } +}