RFR: 8240567: MethodTooLargeException thrown while creating a jlink image [v4]
Oliver Kopp
duke at openjdk.org
Fri Jun 16 10:36:02 UTC 2023
On Mon, 12 Jun 2023 05:03:28 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> Oliver Kopp has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>>
>> - Merge remote-tracking branch 'upstream/master' into fix-8240567
>> - Merge remote-tracking branch 'upstream/master' into fix-8240567
>> - Remove "final" at "enabled" - not part of the point of this PR
>> - Remove comments
>> - 8240567: MethodTooLargeException thrown while creating a jlink image
>>
>> Co-authored-by: Christoph Schwentker <siedlerkiller at gmail.com>
>
> @koppor Is this ready for review? The other PR went through a dozens or so iterations before it was returned to draft. It seems like you were still battling with verifier errors. The comment on this PR says you it was created because a force-push so I can't tell if you the changes are ready or not.
@AlanBateman The diff to the "old" PR https://github.com/openjdk/jdk/pull/10704 is as follows. Maybe this helps at reviewing? IMHO this diff shows very good the new passing of the locals required by the deduplication functionality.
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
old mode 100755
new mode 100644
index 8e98a5b94e2..a87fa84ebe2
--- 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
@@ -118,7 +118,7 @@ public final class SystemModulesPlugin extends AbstractPlugin {
ClassDesc.ofInternalName("jdk/internal/module/SystemModules");
private static final ClassDesc CD_SYSTEM_MODULES_MAP =
ClassDesc.ofInternalName(SYSTEM_MODULES_MAP_CLASSNAME);
- private final boolean enabled;
+ private boolean enabled;
public SystemModulesPlugin() {
super("system-modules");
@@ -519,7 +519,7 @@ public final class SystemModulesPlugin extends AbstractPlugin {
private static final int MAX_LOCAL_VARS = 256;
- private final int BUILDER_VAR = 0;
+ private final int BUILDER_VAR = MAX_LOCAL_VARS + 1;
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
@@ -666,7 +666,7 @@ public final class SystemModulesPlugin extends AbstractPlugin {
* Generate bytecode for moduleDescriptors method
*/
private void genModuleDescriptorsMethod(ClassBuilder clb) {
- if (moduleInfos.size() <= 71) {
+ if (moduleInfos.size() <= 75) {
clb.withMethodBody(
"moduleDescriptors",
MethodTypeDesc.of(CD_MODULE_DESCRIPTOR.arrayType()),
@@ -688,71 +688,98 @@ public final class SystemModulesPlugin extends AbstractPlugin {
return;
}
- // split up module infos in "consumable" packages
List<List<ModuleInfo>> splitModuleInfos = new ArrayList<>();
List<ModuleInfo> currentModuleInfos = null;
for (int index = 0; index < moduleInfos.size(); index++) {
- // The method is "manually split" based on the heuristics that 90 ModuleDescriptors are smaller than 64kb
- // The number 10 is chosen "randomly" to be below the 64kb limit of a method
- if (index % 10 == 0) {
- // Prepare new list
+ if (index % 50 == 0) {
currentModuleInfos = new ArrayList<>();
splitModuleInfos.add(currentModuleInfos);
}
currentModuleInfos.add(moduleInfos.get(index));
}
- // generate all helper methods
+
final String helperMethodNamePrefix = "moduleDescriptorsSub";
+ final ClassDesc arrayListClassDesc = ClassDesc.ofInternalName("java/util/ArrayList");
+
+ final int firstVariableForDedup = nextLocalVar;
+
+ clb.withMethodBody(
+ "moduleDescriptors",
+ MethodTypeDesc.of(CD_MODULE_DESCRIPTOR.arrayType()),
+ ACC_PUBLIC,
+ cob -> {
+ cob.constantInstruction(moduleInfos.size())
+ .anewarray(CD_MODULE_DESCRIPTOR)
+ .dup()
+ .astore(MD_VAR);
+ cob.new_(arrayListClassDesc)
+ .dup()
+ .invokespecial(arrayListClassDesc, "<init>", MethodTypeDesc.of(CD_void))
+ .astore(nextLocalVar);
+ cob.aload(0)
+ .aload(MD_VAR)
+ .aload(nextLocalVar)
+ .invokevirtual(
+ this.classDesc,
+ helperMethodNamePrefix + "0",
+ MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType(), arrayListClassDesc)
+ )
+ .areturn();
+ });
+
final int[] globalCount = {0};
for (final int[] index = {0}; index[0] < splitModuleInfos.size(); index[0]++) {
clb.withMethodBody(
helperMethodNamePrefix + index[0],
- MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType()),
+ MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType(), arrayListClassDesc),
ACC_PUBLIC,
cob -> {
List<ModuleInfo> moduleInfosPackage = splitModuleInfos.get(index[0]);
- if (index[0] > 0) {
- // call last helper method
- cob.aload(0)
- .aload(MD_VAR) // load first parameter, which is MD_VAR
- .invokevirtual(
- this.classDesc,
- helperMethodNamePrefix + (index[0] - 1),
- MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType())
- );
+
+ if (nextLocalVar > firstVariableForDedup) {
+ for (int i = nextLocalVar-1; i >= firstVariableForDedup; i--) {
+ cob.aload(2)
+ .constantInstruction(i-firstVariableForDedup)
+ .invokevirtual(arrayListClassDesc, "get", MethodTypeDesc.of(CD_Object, CD_int))
+ .astore(i);
+ }
}
+
for (int j = 0; j < moduleInfosPackage.size(); j++) {
ModuleInfo minfo = moduleInfosPackage.get(j);
- // executed after the call, thus it is OK to overwrite index 0 (BUILDER_VAR)
new ModuleDescriptorBuilder(cob,
minfo.descriptor(),
minfo.packages(),
globalCount[0]).build();
globalCount[0]++;
}
+
+ if (index[0] + 1 < (splitModuleInfos.size())) {
+ if (nextLocalVar > firstVariableForDedup) {
+ cob.new_(arrayListClassDesc)
+ .dup()
+ .invokespecial(arrayListClassDesc, "<init>", MethodTypeDesc.of(CD_void))
+ .astore(nextLocalVar);
+ for (int i = firstVariableForDedup; i < nextLocalVar; i++) {
+ cob.aload(nextLocalVar)
+ .aload(i)
+ .invokevirtual(arrayListClassDesc, "add", MethodTypeDesc.of(CD_boolean, CD_Object))
+ .pop();
+ }
+ }
+ cob.aload(0)
+ .aload(MD_VAR)
+ .aload(nextLocalVar)
+ .invokevirtual(
+ this.classDesc,
+ helperMethodNamePrefix + (index[0] + 1),
+ MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType(), arrayListClassDesc)
+ );
+ }
+
cob.return_();
});
}
-
- // generate call to last helper method
- clb.withMethodBody(
- "moduleDescriptors",
- MethodTypeDesc.of(CD_MODULE_DESCRIPTOR.arrayType()),
- ACC_PUBLIC,
- cob -> {
- cob.constantInstruction(moduleInfos.size())
- .anewarray(CD_MODULE_DESCRIPTOR)
- .astore(MD_VAR)
- .aload(MD_VAR) // storing for the return at the end
- .aload(0)
- .aload(MD_VAR)
- .invokevirtual(
- this.classDesc,
- helperMethodNamePrefix + (splitModuleInfos.size() - 1),
- MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType())
- )
- .areturn();
- });
}
/**
diff --git a/test/jdk/tools/jlink/JLink100Modules.java b/test/jdk/tools/jlink/JLink100Modules.java
index 4a6111505d2..e71206c904f 100644
--- a/test/jdk/tools/jlink/JLink100Modules.java
+++ b/test/jdk/tools/jlink/JLink100Modules.java
@@ -27,8 +27,10 @@ import java.nio.file.Paths;
import java.util.Arrays;
import java.util.StringJoiner;
import java.util.spi.ToolProvider;
+
import tests.JImageGenerator;
import tests.JImageGenerator.JLinkTask;
+
/*
* @test
* @summary Make sure that 100 modules can be linked using jlink.
@@ -46,9 +48,9 @@ import tests.JImageGenerator.JLinkTask;
*/
public class JLink100Modules {
private static final ToolProvider JAVAC_TOOL = ToolProvider.findFirst("javac")
- .orElseThrow(() -> new RuntimeException("javac tool not found"));
+ .orElseThrow(() -> new RuntimeException("javac tool not found"));
private static final ToolProvider JLINK_TOOL = ToolProvider.findFirst("jlink")
- .orElseThrow(() -> new RuntimeException("jlink tool not found"));
+ .orElseThrow(() -> new RuntimeException("jlink tool not found"));
static void report(String command, String[] args) {
System.out.println(command + " " + String.join(" ", Arrays.asList(args)));
@@ -77,9 +79,10 @@ public class JLink100Modules {
StringBuilder builder = new StringBuilder("module ");
builder.append(name).append(" {");
- for (int j = 0; j < i; j++) {
- builder.append("requires module" + j + "x;");
+ if (i != 0) {
+ builder.append("requires module0x;");
}
+
builder.append("}\n");
Files.writeString(moduleDir.resolve("module-info.java"), builder.toString());
mainModuleInfoContent.add(name);
@@ -106,7 +109,7 @@ public class JLink100Modules {
String out = src.resolve("out").toString();
- javac(new String[] {
+ javac(new String[]{
"-d", out,
"--module-source-path", src.toString(),
"--module", "bug8240567x"
@@ -116,6 +119,7 @@ public class JLink100Modules {
.modulePath(out)
.output(src.resolve("out-jlink"))
.addMods("bug8240567x")
- .call().assertSuccess();
+ .call()
+ .assertSuccess();
}
}
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14408#issuecomment-1594471485
More information about the core-libs-dev
mailing list