From db79e46f7309f35dec42b57e9fd544565174df1f Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Mon, 16 Sep 2024 08:44:24 -0700 Subject: [PATCH 01/15] 8321413: IllegalArgumentException: Code length outside the allowed range while creating a jlink image --- .../internal/plugins/SystemModulesPlugin.java | 546 ++++++++++-------- test/jdk/tools/jlink/JLink3500Packages.java | 126 ++++ 2 files changed, 416 insertions(+), 256 deletions(-) create mode 100644 test/jdk/tools/jlink/JLink3500Packages.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 1a9404b210d5e..d35ca59c743d8 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 @@ -29,7 +29,6 @@ import java.io.IOException; import java.io.InputStream; import java.lang.constant.ClassDesc; -import java.lang.constant.ConstantDesc; import static java.lang.constant.ConstantDescs.*; import java.lang.constant.MethodTypeDesc; import java.lang.module.Configuration; @@ -60,7 +59,8 @@ import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; -import java.util.function.IntSupplier; +import java.util.function.BiConsumer; +import java.util.function.Consumer; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -545,9 +545,7 @@ static class SystemModulesClassGenerator { 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 final int DEDUP_LIST_VAR = 2; - private final int BUILDER_VAR = 3; - private int nextLocalVar = 4; // index to next local variable + private final int BUILDER_VAR = 2; // name of class to generate private final ClassDesc classDesc; @@ -557,11 +555,12 @@ static class SystemModulesClassGenerator { private final int moduleDescriptorsPerMethod; + private final ArrayList> amendaments = new ArrayList<>(); + // 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 - private final DedupSetBuilder dedupSetBuilder - = new DedupSetBuilder(this::getNextLocalVar); + private final DedupSetBuilder dedupSetBuilder; public SystemModulesClassGenerator(String className, List moduleInfos, @@ -569,13 +568,10 @@ public SystemModulesClassGenerator(String className, this.classDesc = ClassDesc.ofInternalName(className); this.moduleInfos = moduleInfos; this.moduleDescriptorsPerMethod = moduleDescriptorsPerMethod; + this.dedupSetBuilder = new DedupSetBuilder(this.classDesc); moduleInfos.forEach(mi -> dedups(mi.descriptor())); } - private int getNextLocalVar() { - return nextLocalVar++; - } - /* * Adds the given ModuleDescriptor to the system module list. * It performs link-time validation and prepares mapping from various @@ -616,6 +612,9 @@ public byte[] genClassBytes(Configuration cf) { // generate genConstructor(clb); + // generate dedup set fields and provider methods + genConstants(clb); + // generate hasSplitPackages genHasSplitPackages(clb); @@ -636,9 +635,16 @@ public byte[] genClassBytes(Configuration cf) { // generate moduleReads genModuleReads(clb, cf); + + // generate module helpers + amendaments.forEach(amendament -> amendament.accept(clb)); }); } + private void addModuleHelpers(Consumer amendament) { + amendaments.add(amendament); + } + /** * Generate bytecode for no-arg constructor */ @@ -654,6 +660,20 @@ private void genConstructor(ClassBuilder clb) { .return_()); } + private void genConstants(ClassBuilder clb) { + var cinitSnippets = dedupSetBuilder.buildConstants(clb); + if (!cinitSnippets.isEmpty()) { + clb.withMethodBody( + CLASS_INIT_NAME, + MTD_void, + ACC_PUBLIC | ACC_STATIC, + cob -> { + cinitSnippets.forEach(snippet -> snippet.accept(cob)); + cob.return_(); + }); + } + } + /** * Generate bytecode for hasSplitPackages method */ @@ -757,7 +777,6 @@ private void genModuleDescriptorsMethod(ClassBuilder clb) { } String helperMethodNamePrefix = "sub"; - ClassDesc arrayListClassDesc = ClassDesc.ofInternalName("java/util/ArrayList"); clb.withMethodBody( "moduleDescriptors", @@ -768,41 +787,24 @@ private void genModuleDescriptorsMethod(ClassBuilder clb) { .anewarray(CD_MODULE_DESCRIPTOR) .dup() .astore(MD_VAR); - cob.new_(arrayListClassDesc) - .dup() - .loadConstant(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) + MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType()) ) .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), + MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType()), ACC_PUBLIC, cob -> { - if (curDedupVar > dedupVarStart) { - for (int i = dedupVarStart; i < curDedupVar; i++) { - cob.aload(DEDUP_LIST_VAR) - .loadConstant(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); @@ -813,21 +815,12 @@ private void genModuleDescriptorsMethod(ClassBuilder clb) { } 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) + MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType()) ); } @@ -1041,36 +1034,7 @@ private void generate(ClassBuilder clb, * Generate code to generate an immutable set. */ private void genImmutableSet(CodeBuilder cob, Set set) { - int size = set.size(); - - // use Set.of(Object[]) when there are more than 2 elements - // use Set.of(Object) or Set.of(Object, Object) when fewer - if (size > 2) { - cob.loadConstant(size) - .anewarray(CD_String); - int i = 0; - for (String element : sorted(set)) { - cob.dup() - .loadConstant(i) - .loadConstant(element) - .aastore(); - i++; - } - cob.invokestatic(CD_Set, - "of", - MTD_Set_ObjectArray, - true); - } else { - for (String element : sorted(set)) { - cob.loadConstant(element); - } - var mtdArgs = new ClassDesc[size]; - Arrays.fill(mtdArgs, CD_Object); - cob.invokestatic(CD_Set, - "of", - MethodTypeDesc.of(CD_Set, mtdArgs), - true); - } + loadImmutableSet(cob, set, CodeBuilder::loadConstant); } class ModuleDescriptorBuilder { @@ -1117,6 +1081,8 @@ class ModuleDescriptorBuilder { static final MethodTypeDesc MTD_ModuleDescriptor_int = MethodTypeDesc.of(CD_MODULE_DESCRIPTOR, CD_int); static final MethodTypeDesc MTD_List_ObjectArray = MethodTypeDesc.of(CD_List, CD_Object.arrayType()); + static final int SET_SIZE_THRESHOLD = 512; + final CodeBuilder cob; final ModuleDescriptor md; final Set packages; @@ -1244,9 +1210,8 @@ void requires(Set requires) { * Builder.newRequires(mods, mn, compiledVersion); */ void newRequires(Set mods, String name, String compiledVersion) { - int varIndex = dedupSetBuilder.indexOfRequiresModifiers(cob, mods); - cob.aload(varIndex) - .loadConstant(name); + dedupSetBuilder.loadRequiresModifiers(cob, mods); + cob.loadConstant(name); if (compiledVersion != null) { cob.loadConstant(compiledVersion) .invokestatic(CD_MODULE_BUILDER, @@ -1266,20 +1231,42 @@ void newRequires(Set mods, String name, String compiledVersio * */ void exports(Set exports) { - cob.aload(BUILDER_VAR) - .loadConstant(exports.size()) - .anewarray(CD_EXPORTS); + cob.aload(BUILDER_VAR); + loadExportsArray(exports); + cob.invokevirtual(CD_MODULE_BUILDER, + "exports", + MTD_EXPORTS_ARRAY) + .pop(); + } + + void loadExportsArray(Set exports) { + if (exports.size() > SET_SIZE_THRESHOLD) { + String methodName = "module" + index + "Exports"; + addModuleHelpers(clb -> clb.withMethodBody( + methodName, + MethodTypeDesc.of(CD_EXPORTS.arrayType()), + ACC_PRIVATE | ACC_FINAL | ACC_STATIC, + mcob -> { + genExportSet(mcob, exports); + mcob.areturn(); + })); + cob.invokestatic(classDesc, methodName, MethodTypeDesc.of(CD_EXPORTS.arrayType())); + } else { + genExportSet(cob, exports); + } + } + + void genExportSet(CodeBuilder cb, Set exports) { + cb.loadConstant(exports.size()) + .anewarray(CD_EXPORTS); int arrayIndex = 0; + for (Exports export : sorted(exports)) { - cob.dup() // arrayref - .loadConstant(arrayIndex++); - newExports(export.modifiers(), export.source(), export.targets()); - cob.aastore(); + cb.dup() // arrayref + .loadConstant(arrayIndex++); + newExports(cb, export.modifiers(), export.source(), export.targets()); + cb.aastore(); } - cob.invokevirtual(CD_MODULE_BUILDER, - "exports", - MTD_EXPORTS_ARRAY) - .pop(); } /* @@ -1297,20 +1284,16 @@ void exports(Set exports) { * Set mods = ... * Builder.newExports(mods, pn, targets); */ - void newExports(Set ms, String pn, Set targets) { - int modifiersSetIndex = dedupSetBuilder.indexOfExportsModifiers(cob, ms); + void newExports(CodeBuilder cb, Set ms, String pn, Set targets) { + dedupSetBuilder.loadExportsModifiers(cb, ms); + cb.loadConstant(pn); if (!targets.isEmpty()) { - int stringSetIndex = dedupSetBuilder.indexOfStringSet(cob, targets); - cob.aload(modifiersSetIndex) - .loadConstant(pn) - .aload(stringSetIndex) - .invokestatic(CD_MODULE_BUILDER, - "newExports", - MTD_EXPORTS_MODIFIER_SET_STRING_SET); + dedupSetBuilder.loadStringSet(cb, targets); + cb.invokestatic(CD_MODULE_BUILDER, + "newExports", + MTD_EXPORTS_MODIFIER_SET_STRING_SET); } else { - cob.aload(modifiersSetIndex) - .loadConstant(pn) - .invokestatic(CD_MODULE_BUILDER, + cb.invokestatic(CD_MODULE_BUILDER, "newExports", MTD_EXPORTS_MODIFIER_SET_STRING); } @@ -1355,19 +1338,15 @@ void opens(Set opens) { * Builder.newOpens(mods, pn, targets); */ void newOpens(Set ms, String pn, Set targets) { - int modifiersSetIndex = dedupSetBuilder.indexOfOpensModifiers(cob, ms); + dedupSetBuilder.loadOpensModifiers(cob, ms); + cob.loadConstant(pn); if (!targets.isEmpty()) { - int stringSetIndex = dedupSetBuilder.indexOfStringSet(cob, targets); - cob.aload(modifiersSetIndex) - .loadConstant(pn) - .aload(stringSetIndex) - .invokestatic(CD_MODULE_BUILDER, + dedupSetBuilder.loadStringSet(cob, targets); + cob.invokestatic(CD_MODULE_BUILDER, "newOpens", MTD_OPENS_MODIFIER_SET_STRING_SET); } else { - cob.aload(modifiersSetIndex) - .loadConstant(pn) - .invokestatic(CD_MODULE_BUILDER, + cob.invokestatic(CD_MODULE_BUILDER, "newOpens", MTD_OPENS_MODIFIER_SET_STRING); } @@ -1377,10 +1356,9 @@ void newOpens(Set ms, String pn, Set targets) { * Invoke Builder.uses(Set uses) */ void uses(Set uses) { - int varIndex = dedupSetBuilder.indexOfStringSet(cob, uses); - cob.aload(BUILDER_VAR) - .aload(varIndex) - .invokevirtual(CD_MODULE_BUILDER, + cob.aload(BUILDER_VAR); + dedupSetBuilder.loadStringSet(cob, uses); + cob.invokevirtual(CD_MODULE_BUILDER, "uses", MTD_SET) .pop(); @@ -1442,10 +1420,24 @@ void newProvides(String service, List providers) { * Invoke Builder.packages(String pn) */ void packages(Set packages) { - int varIndex = dedupSetBuilder.newStringSet(cob, packages); - cob.aload(BUILDER_VAR) - .aload(varIndex) - .invokevirtual(CD_MODULE_BUILDER, + cob.aload(BUILDER_VAR); + if (packages.size() > SET_SIZE_THRESHOLD) { + var methodName = "module" + index + "Packages"; + addModuleHelpers(clb -> { + clb.withMethodBody( + methodName, + MethodTypeDesc.of(CD_Set), + ACC_PRIVATE | ACC_FINAL | ACC_STATIC, + cob -> { + genImmutableSet(cob, packages); + cob.areturn(); + }); + }); + cob.invokestatic(classDesc, methodName, MethodTypeDesc.of(CD_Set)); + } else { + genImmutableSet(cob, packages); + } + cob.invokevirtual(CD_MODULE_BUILDER, "packages", MTD_SET) .pop(); @@ -1583,31 +1575,34 @@ void hashForModule(String name, byte[] hash) { static class DedupSetBuilder { // map Set to a specialized builder to allow them to be // deduplicated as they are requested - final Map, SetBuilder> stringSets = new HashMap<>(); + final Map, SetReference> stringSets = new HashMap<>(); // map Set to a specialized builder to allow them to be // deduplicated as they are requested - final Map, EnumSetBuilder> + final Map, SetReference> requiresModifiersSets = new HashMap<>(); // map Set to a specialized builder to allow them to be // deduplicated as they are requested - final Map, EnumSetBuilder> + final Map, SetReference> exportsModifiersSets = new HashMap<>(); // map Set to a specialized builder to allow them to be // deduplicated as they are requested - final Map, EnumSetBuilder> + final Map, SetReference> opensModifiersSets = new HashMap<>(); - private final int stringSetVar; - private final int enumSetVar; - private final IntSupplier localVarSupplier; + private static final String VALUES_ARRAY = "dedupSetValues"; - DedupSetBuilder(IntSupplier localVarSupplier) { - this.stringSetVar = localVarSupplier.getAsInt(); - this.enumSetVar = localVarSupplier.getAsInt(); - this.localVarSupplier = localVarSupplier; + int counterStoredValues = 0; + ClassDesc owner; + + DedupSetBuilder(ClassDesc owner) { + this.owner = owner; + } + + int requestValueStorage() { + return counterStoredValues++; } /* @@ -1615,7 +1610,7 @@ static class DedupSetBuilder { */ void stringSet(Set strings) { stringSets.computeIfAbsent(strings, - s -> new SetBuilder<>(s, stringSetVar, localVarSupplier) + s -> new SetReference<>(s, CodeBuilder::loadConstant) ).increment(); } @@ -1624,8 +1619,7 @@ void stringSet(Set strings) { */ void exportsModifiers(Set mods) { exportsModifiersSets.computeIfAbsent(mods, s -> - new EnumSetBuilder<>(s, CD_EXPORTS_MODIFIER, - enumSetVar, localVarSupplier) + new SetReference<>(s, getEnumLoader(CD_EXPORTS_MODIFIER)) ).increment(); } @@ -1634,8 +1628,7 @@ void exportsModifiers(Set mods) { */ void opensModifiers(Set mods) { opensModifiersSets.computeIfAbsent(mods, s -> - new EnumSetBuilder<>(s, CD_OPENS_MODIFIER, - enumSetVar, localVarSupplier) + new SetReference<>(s, getEnumLoader(CD_OPENS_MODIFIER)) ).increment(); } @@ -1644,174 +1637,215 @@ void opensModifiers(Set mods) { */ void requiresModifiers(Set mods) { requiresModifiersSets.computeIfAbsent(mods, s -> - new EnumSetBuilder<>(s, CD_REQUIRES_MODIFIER, - enumSetVar, localVarSupplier) + new SetReference<>(s, getEnumLoader(CD_REQUIRES_MODIFIER)) ).increment(); } /* - * Retrieve the index to the given set of Strings. Emit code to - * generate it when SetBuilder::build is called. + * Load the given set to the top of operand stack. */ - int indexOfStringSet(CodeBuilder cob, Set names) { - return stringSets.get(names).build(cob); + void loadStringSet(CodeBuilder cob, Set names) { + stringSets.get(names).load(cob); } /* - * Retrieve the index to the given set of Exports.Modifier. - * Emit code to generate it when EnumSetBuilder::build is called. + * Load the given set to the top of operand stack. */ - int indexOfExportsModifiers(CodeBuilder cob, Set mods) { - return exportsModifiersSets.get(mods).build(cob); + void loadExportsModifiers(CodeBuilder cob, Set mods) { + exportsModifiersSets.get(mods).load(cob); } - /** - * Retrieve the index to the given set of Opens.Modifier. - * Emit code to generate it when EnumSetBuilder::build is called. + /* + * Load the given set to the top of operand stack. */ - int indexOfOpensModifiers(CodeBuilder cob, Set mods) { - return opensModifiersSets.get(mods).build(cob); + void loadOpensModifiers(CodeBuilder cob, Set mods) { + opensModifiersSets.get(mods).load(cob); } /* - * Retrieve the index to the given set of Requires.Modifier. - * Emit code to generate it when EnumSetBuilder::build is called. + * Load the given set to the top of operand stack. */ - int indexOfRequiresModifiers(CodeBuilder cob, Set mods) { - return requiresModifiersSets.get(mods).build(cob); + void loadRequiresModifiers(CodeBuilder cob, Set mods) { + requiresModifiersSets.get(mods).load(cob); } /* - * Build a new string set without any attempt to deduplicate it. + * Adding provider methods to the class. For those set used more than once, built + * once and keep the reference for later access. + * Return a list of snippet to be used in . */ - int newStringSet(CodeBuilder cob, Set names) { - int index = new SetBuilder<>(names, stringSetVar, localVarSupplier).build(cob); - assert index == stringSetVar; - return index; - } - } - - /* - * SetBuilder generates bytecode to create one single instance of Set - * for a given set of elements and assign to a local variable slot. - * When there is only one single reference to a Set, - * it will reuse defaultVarIndex. For a Set with multiple references, - * it will use a new local variable retrieved from the nextLocalVar - */ - static class SetBuilder> { - private static final MethodTypeDesc MTD_Set_ObjectArray = MethodTypeDesc.of( - CD_Set, CD_Object.arrayType()); + Collection> buildConstants(ClassBuilder clb) { + var index = 0; + ArrayList> setValueBuilders = new ArrayList<>(); - private final Set elements; - private final int defaultVarIndex; - private final IntSupplier nextLocalVar; - private int refCount; - private int localVarIndex; + for (var ref : sorted(stringSets.values())) { + index++; + ref.generateConstant(clb, "dedupStringSet" + index).ifPresent(setValueBuilders::add); + } + for (var ref: sorted(opensModifiersSets.values())) { + index++; + ref.generateConstant(clb, "dedupOpensSet" + index).ifPresent(setValueBuilders::add); + } + for (var ref: sorted(exportsModifiersSets.values())) { + index++; + ref.generateConstant(clb, "dedupExportsSet" + index).ifPresent(setValueBuilders::add); + } + for (var ref: sorted(requiresModifiersSets.values())) { + index++; + ref.generateConstant(clb, "dedupRequiresSet" + index).ifPresent(setValueBuilders::add); + } - SetBuilder(Set elements, - int defaultVarIndex, - IntSupplier nextLocalVar) { - this.elements = elements; - this.defaultVarIndex = defaultVarIndex; - this.nextLocalVar = nextLocalVar; + if (counterStoredValues > 0) { + assert setValueBuilders.size() == counterStoredValues; + clb.withField(VALUES_ARRAY, CD_Set.arrayType(), ACC_STATIC | ACC_FINAL); + setValueBuilders.addFirst(cob -> + cob.loadConstant(counterStoredValues) + .anewarray(CD_Set) + .putstatic(owner, VALUES_ARRAY, CD_Set.arrayType())); + } + return setValueBuilders; } - /* - * Increments the number of references to this particular set. - */ - final void increment() { - refCount++; + void loadValuesArray(CodeBuilder cob) { + cob.getstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); } - /** - * Generate the appropriate instructions to load an object reference - * to the element onto the stack. - */ - void visitElement(T element, CodeBuilder cob) { - cob.loadConstant((ConstantDesc)element); - } + class SetReference> implements Comparable> { + private final Set elements; + private final BiConsumer elementLoader; - /* - * Build bytecode for the Set represented by this builder, - * or get the local variable index of a previously generated set - * (in the local scope). - * - * @return local variable index of the generated set. - */ - final int build(CodeBuilder cob) { - int index = localVarIndex; - if (localVarIndex == 0) { - // if non-empty and more than one set reference this builder, - // emit to a unique local - index = refCount <= 1 ? defaultVarIndex - : nextLocalVar.getAsInt(); - if (index < MAX_LOCAL_VARS) { - localVarIndex = index; + private int refCount; + private int index = -1; + private String methodName; + + SetReference(Set elements, BiConsumer elementLoader) { + this.elements = elements; + this.elementLoader = elementLoader; + } + + int increment() { + return ++refCount; + } + + + // Load the set to the operand stack + void load(CodeBuilder cob) { + if (refCount > 1) { + assert index >= 0; + loadValuesArray(cob); + cob.loadConstant(index); + cob.aaload(); } else { - // overflow: disable optimization by using localVarIndex = 0 - index = defaultVarIndex; + build(cob); } + } - generateSetOf(cob, index); + // Build the set value and store the reference + void store(CodeBuilder cob) { + assert index >= 0; + loadValuesArray(cob); + cob.loadConstant(index); + build(cob); + cob.aastore(); } - return index; - } - private void generateSetOf(CodeBuilder cob, int index) { - if (elements.size() <= 10) { - // call Set.of(e1, e2, ...) - for (T t : sorted(elements)) { - visitElement(t, cob); + // build the set and leave the reference at top of the operand stack + private void build(CodeBuilder cob) { + if (methodName != null) { + cob.invokestatic(owner, methodName, MethodTypeDesc.of(CD_Set)); + } else { + loadImmutableSet(cob, elements, elementLoader); } - var mtdArgs = new ClassDesc[elements.size()]; - Arrays.fill(mtdArgs, CD_Object); - cob.invokestatic(CD_Set, - "of", - MethodTypeDesc.of(CD_Set, mtdArgs), - true); - } else { - // call Set.of(E... elements) - cob.loadConstant(elements.size()) - .anewarray(CD_String); - int arrayIndex = 0; - for (T t : sorted(elements)) { - cob.dup() // arrayref - .loadConstant(arrayIndex); - visitElement(t, cob); // value - cob.aastore(); - arrayIndex++; + } + + /** + * Generate provider method if the set size is over threshold to avoid overload + * bytecode limitation per method. + * Return a snippet builder that generates code to store the reference of the set value. + */ + Optional> generateConstant(ClassBuilder clb, String name) { + if (elements.size() > ModuleDescriptorBuilder.SET_SIZE_THRESHOLD) { + methodName = name + "Provider"; + genImmutableSetProvider(clb, methodName, elements, elementLoader); + } + + if (refCount <= 1) { + return Optional.empty(); + } else { + index = requestValueStorage(); + return Optional.of(this::store); + } + } + + @Override + public int compareTo(SetReference o) { + if (o == this) { + return 0; + } + if (elements.size() == o.elements.size()) { + var a1 = sorted(elements); + var a2 = sorted(o.elements); + for (int i = 0; i < elements.size(); i++) { + var r = a1.get(i).compareTo(a2.get(i)); + if (r != 0) { + return r; + } + } + return 0; + } else { + return elements.size() - o.elements.size(); } - cob.invokestatic(CD_Set, - "of", - MTD_Set_ObjectArray, - true); } - cob.astore(index); } } - /* - * Generates bytecode to create one single instance of EnumSet - * for a given set of modifiers and assign to a local variable slot. - */ - static class EnumSetBuilder> extends SetBuilder { - private final ClassDesc classDesc; + static > BiConsumer getEnumLoader(ClassDesc enumClassDesc) { + return (cob, element) -> cob.getstatic(enumClassDesc, element.toString(), enumClassDesc); + } - EnumSetBuilder(Set modifiers, ClassDesc classDesc, - int defaultVarIndex, - IntSupplier nextLocalVar) { - super(modifiers, defaultVarIndex, nextLocalVar); - this.classDesc = classDesc; + static > void loadImmutableSet(CodeBuilder cob, + Set elements, + BiConsumer elementLoader) { + if (elements.size() <= 10) { + // call Set.of(e1, e2, ...) + for (T t : sorted(elements)) { + elementLoader.accept(cob, t); + } + var mtdArgs = new ClassDesc[elements.size()]; + Arrays.fill(mtdArgs, CD_Object); + cob.invokestatic(CD_Set, + "of", + MethodTypeDesc.of(CD_Set, mtdArgs), + true); + } else { + // call Set.of(E... elements) + cob.loadConstant(elements.size()) + .anewarray(CD_String); + int arrayIndex = 0; + for (T t : sorted(elements)) { + cob.dup() // arrayref + .loadConstant(arrayIndex); + elementLoader.accept(cob, t); // value + cob.aastore(); + arrayIndex++; + } + cob.invokestatic(CD_Set, "of", MTD_Set_ObjectArray, true); } + } - /** - * Loads an Enum field. - */ - @Override - void visitElement(T t, CodeBuilder cob) { - cob.getstatic(classDesc, t.toString(), classDesc); - } + static > void genImmutableSetProvider(ClassBuilder clb, + String methodName, + Set elements, + BiConsumer elementLoader) { + clb.withMethodBody( + methodName, + MethodTypeDesc.of(CD_Set), + ACC_PRIVATE | ACC_FINAL | ACC_STATIC, + cob -> { + loadImmutableSet(cob, elements, elementLoader); + cob.areturn(); + }); } } diff --git a/test/jdk/tools/jlink/JLink3500Packages.java b/test/jdk/tools/jlink/JLink3500Packages.java new file mode 100644 index 0000000000000..bcb4a4ccf95a2 --- /dev/null +++ b/test/jdk/tools/jlink/JLink3500Packages.java @@ -0,0 +1,126 @@ +/* + * 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 4000 packages in a uber jar can be linked using jlink. + * @bug 8321413 + * @library ../lib + * @enablePreview + * @modules java.base/jdk.internal.jimage + * 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 JLink3500Packages + */ +public class JLink3500Packages { + 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("bug8321413"); + Path mainModulePath = src.resolve("bug8321413x"); + + StringJoiner mainModuleInfoContent = new StringJoiner(";\n exports ", "module bug8321413x {\n exports ", ";\n}"); + + for (int i = 0; i < 3500; i++) { + String packageName = "p" + i; + String className = "C" + i; + + Path packagePath = Files.createDirectories(mainModulePath.resolve(packageName)); + + StringBuilder classContent = new StringBuilder("package "); + classContent.append(packageName).append(";\n"); + classContent.append("class ").append(className).append(" {}\n"); + Files.writeString(packagePath.resolve(className + ".java"), classContent.toString()); + + mainModuleInfoContent.add(packageName); + } + + // create module reading the generated modules + Path mainModuleInfo = mainModulePath.resolve("module-info.java"); + Files.writeString(mainModuleInfo, mainModuleInfoContent.toString()); + + Path mainClassDir = mainModulePath.resolve("testpackage"); + Files.createDirectories(mainClassDir); + + Files.writeString(mainClassDir.resolve("JLink3500PackagesTest.java"), """ + package testpackage; + + public class JLink3500PackagesTest { + public static void main(String[] args) throws Exception { + System.out.println("JLink3500PackagesTest started."); + } + } + """); + + String out = src.resolve("out").toString(); + javac(new String[]{ + "-d", out, + "--module-source-path", src.toString(), + "--module", "bug8321413x" + }); + + JImageGenerator.getJLinkTask() + .modulePath(out) + .output(src.resolve("out-jlink")) + .addMods("bug8321413x") + .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", "bug8321413x/testpackage.JLink3500PackagesTest"); + processBuilder.inheritIO(); + processBuilder.directory(binDir.toFile()); + Process process = processBuilder.start(); + int exitCode = process.waitFor(); + if (exitCode != 0) + throw new AssertionError("JLink3500PackagesTest failed to launch"); + } +} From ffce5eba2a49e7f682cd7ac351c1e68e47f9fac9 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Thu, 26 Sep 2024 18:14:37 -0700 Subject: [PATCH 02/15] Address review comments --- .../internal/plugins/SystemModulesPlugin.java | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) 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 d35ca59c743d8..37718d8b7ed3c 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 @@ -666,7 +666,7 @@ private void genConstants(ClassBuilder clb) { clb.withMethodBody( CLASS_INIT_NAME, MTD_void, - ACC_PUBLIC | ACC_STATIC, + ACC_STATIC, cob -> { cinitSnippets.forEach(snippet -> snippet.accept(cob)); cob.return_(); @@ -1081,7 +1081,7 @@ class ModuleDescriptorBuilder { static final MethodTypeDesc MTD_ModuleDescriptor_int = MethodTypeDesc.of(CD_MODULE_DESCRIPTOR, CD_int); static final MethodTypeDesc MTD_List_ObjectArray = MethodTypeDesc.of(CD_List, CD_Object.arrayType()); - static final int SET_SIZE_THRESHOLD = 512; + static final int SET_SIZE_THRESHOLD = 512; // An arbitrary number as this likely generate minimum ~4K code final CodeBuilder cob; final ModuleDescriptor md; @@ -1245,7 +1245,7 @@ void loadExportsArray(Set exports) { addModuleHelpers(clb -> clb.withMethodBody( methodName, MethodTypeDesc.of(CD_EXPORTS.arrayType()), - ACC_PRIVATE | ACC_FINAL | ACC_STATIC, + ACC_STATIC, mcob -> { genExportSet(mcob, exports); mcob.areturn(); @@ -1424,15 +1424,15 @@ void packages(Set packages) { if (packages.size() > SET_SIZE_THRESHOLD) { var methodName = "module" + index + "Packages"; addModuleHelpers(clb -> { - clb.withMethodBody( - methodName, - MethodTypeDesc.of(CD_Set), - ACC_PRIVATE | ACC_FINAL | ACC_STATIC, - cob -> { - genImmutableSet(cob, packages); - cob.areturn(); - }); - }); + clb.withMethodBody( + methodName, + MethodTypeDesc.of(CD_Set), + ACC_STATIC, + cob -> { + genImmutableSet(cob, packages); + cob.areturn(); + }); + }); cob.invokestatic(classDesc, methodName, MethodTypeDesc.of(CD_Set)); } else { genImmutableSet(cob, packages); @@ -1594,8 +1594,8 @@ static class DedupSetBuilder { private static final String VALUES_ARRAY = "dedupSetValues"; + final ClassDesc owner; int counterStoredValues = 0; - ClassDesc owner; DedupSetBuilder(ClassDesc owner) { this.owner = owner; @@ -1801,7 +1801,7 @@ public int compareTo(SetReference o) { } static > BiConsumer getEnumLoader(ClassDesc enumClassDesc) { - return (cob, element) -> cob.getstatic(enumClassDesc, element.toString(), enumClassDesc); + return (cob, element) -> cob.getstatic(enumClassDesc, element.name(), enumClassDesc); } static > void loadImmutableSet(CodeBuilder cob, @@ -1814,18 +1814,15 @@ static > void loadImmutableSet(CodeBuilder cob, } var mtdArgs = new ClassDesc[elements.size()]; Arrays.fill(mtdArgs, CD_Object); - cob.invokestatic(CD_Set, - "of", - MethodTypeDesc.of(CD_Set, mtdArgs), - true); + cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, mtdArgs), true); } else { // call Set.of(E... elements) cob.loadConstant(elements.size()) - .anewarray(CD_String); + .anewarray(CD_String); int arrayIndex = 0; for (T t : sorted(elements)) { cob.dup() // arrayref - .loadConstant(arrayIndex); + .loadConstant(arrayIndex); elementLoader.accept(cob, t); // value cob.aastore(); arrayIndex++; @@ -1841,7 +1838,7 @@ static > void genImmutableSetProvider(ClassBuilder clb, clb.withMethodBody( methodName, MethodTypeDesc.of(CD_Set), - ACC_PRIVATE | ACC_FINAL | ACC_STATIC, + ACC_STATIC, cob -> { loadImmutableSet(cob, elements, elementLoader); cob.areturn(); From 858703ee00ab40842e8143f3f777c192ad97bf8a Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Fri, 27 Sep 2024 11:00:22 -0700 Subject: [PATCH 03/15] Missing from last commit --- test/jdk/tools/jlink/JLink3500Packages.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/jdk/tools/jlink/JLink3500Packages.java b/test/jdk/tools/jlink/JLink3500Packages.java index bcb4a4ccf95a2..917ba439e001a 100644 --- a/test/jdk/tools/jlink/JLink3500Packages.java +++ b/test/jdk/tools/jlink/JLink3500Packages.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 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 @@ -32,7 +32,9 @@ /* * @test - * @summary Make sure that 4000 packages in a uber jar can be linked using jlink. + * @summary Make sure that ~3500 packages in a uber jar can be linked using jlink. Depends on the + * packages, this is almost hit the 64K limitation as each plain export could take + * ~17 bytecodes. * @bug 8321413 * @library ../lib * @enablePreview From b2794018d7f14d2e4cc4712e4477727474df76ef Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Fri, 4 Oct 2024 10:39:38 -0700 Subject: [PATCH 04/15] Fix typo and add comments based on review feedback --- .../internal/plugins/SystemModulesPlugin.java | 67 +++++++++---------- 1 file changed, 30 insertions(+), 37 deletions(-) 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 37718d8b7ed3c..d60a6c789898e 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 @@ -555,7 +555,7 @@ static class SystemModulesClassGenerator { private final int moduleDescriptorsPerMethod; - private final ArrayList> amendaments = new ArrayList<>(); + private final ArrayList> amendments = new ArrayList<>(); // A builder to create one single Set instance for a given set of // names or modifiers to reduce the footprint @@ -637,12 +637,12 @@ public byte[] genClassBytes(Configuration cf) { genModuleReads(clb, cf); // generate module helpers - amendaments.forEach(amendament -> amendament.accept(clb)); + amendments.forEach(amendment -> amendment.accept(clb)); }); } - private void addModuleHelpers(Consumer amendament) { - amendaments.add(amendament); + private void addModuleHelpers(Consumer amendment) { + amendments.add(amendment); } /** @@ -661,14 +661,14 @@ private void genConstructor(ClassBuilder clb) { } private void genConstants(ClassBuilder clb) { - var cinitSnippets = dedupSetBuilder.buildConstants(clb); - if (!cinitSnippets.isEmpty()) { + var clinitSnippets = dedupSetBuilder.buildConstants(clb); + if (!clinitSnippets.isEmpty()) { clb.withMethodBody( CLASS_INIT_NAME, MTD_void, ACC_STATIC, cob -> { - cinitSnippets.forEach(snippet -> snippet.accept(cob)); + clinitSnippets.forEach(snippet -> snippet.accept(cob)); cob.return_(); }); } @@ -743,29 +743,7 @@ private void genModuleDescriptorsMethod(ClassBuilder clb) { // 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); - // } - + // and so on. List> splitModuleInfos = new ArrayList<>(); List currentModuleInfos = null; for (int index = 0; index < moduleInfos.size(); index++) { @@ -1595,14 +1573,14 @@ static class DedupSetBuilder { private static final String VALUES_ARRAY = "dedupSetValues"; final ClassDesc owner; - int counterStoredValues = 0; + int countOfStoredValues = 0; DedupSetBuilder(ClassDesc owner) { this.owner = owner; } int requestValueStorage() { - return counterStoredValues++; + return countOfStoredValues++; } /* @@ -1673,12 +1651,12 @@ void loadRequiresModifiers(CodeBuilder cob, Set mods) { /* * Adding provider methods to the class. For those set used more than once, built * once and keep the reference for later access. - * Return a list of snippet to be used in . + * Return a list of snippet to be used in . */ Collection> buildConstants(ClassBuilder clb) { var index = 0; ArrayList> setValueBuilders = new ArrayList<>(); - + // The SetReferences need to be sorted to reproduce same result. for (var ref : sorted(stringSets.values())) { index++; ref.generateConstant(clb, "dedupStringSet" + index).ifPresent(setValueBuilders::add); @@ -1696,11 +1674,12 @@ Collection> buildConstants(ClassBuilder clb) { ref.generateConstant(clb, "dedupRequiresSet" + index).ifPresent(setValueBuilders::add); } - if (counterStoredValues > 0) { - assert setValueBuilders.size() == counterStoredValues; + if (countOfStoredValues > 0) { + assert setValueBuilders.size() == countOfStoredValues; clb.withField(VALUES_ARRAY, CD_Set.arrayType(), ACC_STATIC | ACC_FINAL); + // Allocate array before assign values setValueBuilders.addFirst(cob -> - cob.loadConstant(counterStoredValues) + cob.loadConstant(countOfStoredValues) .anewarray(CD_Set) .putstatic(owner, VALUES_ARRAY, CD_Set.arrayType())); } @@ -1711,6 +1690,20 @@ void loadValuesArray(CodeBuilder cob) { cob.getstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); } + /* + * SetReference count references to the set, and use a CodeBuilder that + * generate bytecode to load an element onto the operand stack to generate bytecode + * to support loading the set onto operand stack. + * + * When a set size is over SET_SIZE_THRESHOLD, a provider function is generated + * to build the set rather than inline to avoid method size overflow. + * + * When a set is referenced more than once, the set value is to be built once + * and cached in an array to be load later. + * + * generateConstant method should be called to setup the provider methods and cache array. + * load method can then be called to load the set onto the operand stack. + */ class SetReference> implements Comparable> { private final Set elements; private final BiConsumer elementLoader; From ab6459d3d3d3ce2349412bf303c8ac94cf3100fc Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Mon, 14 Oct 2024 21:51:33 -0700 Subject: [PATCH 05/15] Add more comments, move snippet generation code into standalone class --- .../jdk/tools/jlink/internal/Snippets.java | 135 +++++++++ .../internal/plugins/SystemModulesPlugin.java | 260 +++++++----------- 2 files changed, 234 insertions(+), 161 deletions(-) create mode 100644 src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java new file mode 100644 index 0000000000000..367a72b0cc6b0 --- /dev/null +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -0,0 +1,135 @@ +/* + * Copyright (c) 2024, 2024, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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 jdk.tools.jlink.internal; + +import java.lang.classfile.ClassBuilder; +import static java.lang.classfile.ClassFile.ACC_STATIC; +import java.lang.classfile.CodeBuilder; +import java.lang.constant.ClassDesc; +import static java.lang.constant.ConstantDescs.CD_Object; +import static java.lang.constant.ConstantDescs.CD_Set; +import java.lang.constant.MethodTypeDesc; +import java.util.Arrays; +import java.util.Collection; +import java.util.function.BiConsumer; + +public class Snippets { + /** + * Return a snippet builder that loads an enum onto the operand stack using + * the enum name static final field + */ + public static > BiConsumer getEnumLoader(ClassDesc enumClassDesc) { + return (cob, element) -> cob.getstatic(enumClassDesc, element.name(), enumClassDesc); + } + + /** + * Generate bytecode to create an array and load onto the operand stack. + * Effectively like following pseudo code: + * new T[] { elements } + * + * @param cob The code builder to add the snipper + * @param elementType The class descriptor of the element type T + * @param elements The elements to be in the array + * @param elementLoader A snippet generator to load an element T onto the operand stack. + */ + public static void loadArray(CodeBuilder cob, + ClassDesc elementType, + Collection elements, + BiConsumer elementLoader) { + cob.loadConstant(elements.size()) + .anewarray(elementType); + int arrayIndex = 0; + for (T t : elements) { + cob.dup() // arrayref + .loadConstant(arrayIndex); + elementLoader.accept(cob, t); // value + cob.aastore(); + arrayIndex++; + } + } + + /** + * Generates bytecode to load a set onto the operand stack. + * Effectively like following pseudo code: + * Set.of(elements) + * @param cob The code builder to add the snippet + * @param elements The set to be created + * @param elementLoader Snippet generator to load an element onto the operand stack + */ + public static void loadImmutableSet(CodeBuilder cob, + Collection elements, + BiConsumer elementLoader) { + if (elements.size() <= 10) { + // call Set.of(e1, e2, ...) + for (T t : elements) { + elementLoader.accept(cob, t); + } + var mtdArgs = new ClassDesc[elements.size()]; + Arrays.fill(mtdArgs, CD_Object); + cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, mtdArgs), true); + } else { + // call Set.of(E... elements) + loadArray(cob, CD_Object, elements, elementLoader); + cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, CD_Object.arrayType()), true); + } + } + + // Generate a method with pseudo code looks like this + // static Set methodName() { + // return Set.of(elements); + // } + public static void genImmutableSetProvider(ClassBuilder clb, + String methodName, + Collection elements, + BiConsumer elementLoader) { + clb.withMethodBody( + methodName, + MethodTypeDesc.of(CD_Set), + ACC_STATIC, + cob -> { + loadImmutableSet(cob, elements, elementLoader); + cob.areturn(); + }); + } + + // Generate a method with pseudo code looks like this + // static T[] methodName() { + // return new T[] { elements }; + // } + public static void genArrayProvider(ClassBuilder clb, + String methodName, + ClassDesc elementType, + Collection elements, + BiConsumer elementLoader) { + clb.withMethodBody( + methodName, + MethodTypeDesc.of(elementType.arrayType()), + ACC_STATIC, + cob -> { + loadArray(cob, elementType, elements, elementLoader); + cob.areturn(); + }); + } +} \ No newline at end of file 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 d60a6c789898e..11e0337f7454a 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 @@ -45,7 +45,6 @@ import java.lang.reflect.ClassFileFormatVersion; import java.net.URI; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.EnumSet; @@ -78,7 +77,6 @@ import java.lang.classfile.ClassBuilder; import java.lang.classfile.ClassFile; import java.lang.classfile.TypeKind; -import static java.lang.classfile.ClassFile.*; import java.lang.classfile.CodeBuilder; import jdk.tools.jlink.internal.ModuleSorter; @@ -87,6 +85,14 @@ import jdk.tools.jlink.plugin.ResourcePoolBuilder; import jdk.tools.jlink.plugin.ResourcePoolEntry; +import static java.lang.classfile.ClassFile.*; + +import static jdk.tools.jlink.internal.Snippets.genArrayProvider; +import static jdk.tools.jlink.internal.Snippets.genImmutableSetProvider; +import static jdk.tools.jlink.internal.Snippets.getEnumLoader; +import static jdk.tools.jlink.internal.Snippets.loadArray; +import static jdk.tools.jlink.internal.Snippets.loadImmutableSet; + /** * Jlink plugin to reconstitute module descriptors and other attributes for system * modules. The plugin generates implementations of SystemModules to avoid parsing @@ -538,7 +544,7 @@ static class SystemModulesClassGenerator { private static final MethodTypeDesc MTD_Map = MethodTypeDesc.of(CD_Map); private static final MethodTypeDesc MTD_MapEntry_Object_Object = MethodTypeDesc.of(CD_Map_Entry, CD_Object, CD_Object); private static final MethodTypeDesc MTD_Map_MapEntryArray = MethodTypeDesc.of(CD_Map, CD_Map_Entry.arrayType()); - private static final MethodTypeDesc MTD_Set_ObjectArray = MethodTypeDesc.of(CD_Set, CD_Object.arrayType()); + private static final int MAX_LOCAL_VARS = 256; @@ -669,7 +675,8 @@ private void genConstants(ClassBuilder clb) { ACC_STATIC, cob -> { clinitSnippets.forEach(snippet -> snippet.accept(cob)); - cob.return_(); + cob.pop() + .return_(); }); } } @@ -1012,7 +1019,7 @@ private void generate(ClassBuilder clb, * Generate code to generate an immutable set. */ private void genImmutableSet(CodeBuilder cob, Set set) { - loadImmutableSet(cob, set, CodeBuilder::loadConstant); + loadImmutableSet(cob, sorted(set), CodeBuilder::loadConstant); } class ModuleDescriptorBuilder { @@ -1220,30 +1227,14 @@ void exports(Set exports) { void loadExportsArray(Set exports) { if (exports.size() > SET_SIZE_THRESHOLD) { String methodName = "module" + index + "Exports"; - addModuleHelpers(clb -> clb.withMethodBody( + addModuleHelpers(clb -> genArrayProvider(clb, methodName, - MethodTypeDesc.of(CD_EXPORTS.arrayType()), - ACC_STATIC, - mcob -> { - genExportSet(mcob, exports); - mcob.areturn(); - })); + CD_EXPORTS, + sorted(exports), + this::loadExports)); cob.invokestatic(classDesc, methodName, MethodTypeDesc.of(CD_EXPORTS.arrayType())); } else { - genExportSet(cob, exports); - } - } - - void genExportSet(CodeBuilder cb, Set exports) { - cb.loadConstant(exports.size()) - .anewarray(CD_EXPORTS); - int arrayIndex = 0; - - for (Exports export : sorted(exports)) { - cb.dup() // arrayref - .loadConstant(arrayIndex++); - newExports(cb, export.modifiers(), export.source(), export.targets()); - cb.aastore(); + loadArray(cob, CD_EXPORTS, sorted(exports), this::loadExports); } } @@ -1254,17 +1245,14 @@ void genExportSet(CodeBuilder cb, Set exports) { * or * Builder.newExports(Set ms, String pn) * - * Set targets = new HashSet<>(); - * targets.add(t); - * : - * : - * - * Set mods = ... - * Builder.newExports(mods, pn, targets); + * ms = export.modifiers() + * pn = export.source() + * targets = export.targets() */ - void newExports(CodeBuilder cb, Set ms, String pn, Set targets) { - dedupSetBuilder.loadExportsModifiers(cb, ms); - cb.loadConstant(pn); + void loadExports(CodeBuilder cb, Exports export) { + dedupSetBuilder.loadExportsModifiers(cb, export.modifiers()); + cb.loadConstant(export.source()); + var targets = export.targets(); if (!targets.isEmpty()) { dedupSetBuilder.loadStringSet(cb, targets); cb.invokestatic(CD_MODULE_BUILDER, @@ -1272,8 +1260,8 @@ void newExports(CodeBuilder cb, Set ms, String pn, Set MTD_EXPORTS_MODIFIER_SET_STRING_SET); } else { cb.invokestatic(CD_MODULE_BUILDER, - "newExports", - MTD_EXPORTS_MODIFIER_SET_STRING); + "newExports", + MTD_EXPORTS_MODIFIER_SET_STRING); } } @@ -1284,16 +1272,8 @@ void newExports(CodeBuilder cb, Set ms, String pn, Set * Builder.opens(Opens[]) */ void opens(Set opens) { - cob.aload(BUILDER_VAR) - .loadConstant(opens.size()) - .anewarray(CD_OPENS); - int arrayIndex = 0; - for (Opens open : sorted(opens)) { - cob.dup() // arrayref - .loadConstant(arrayIndex++); - newOpens(open.modifiers(), open.source(), open.targets()); - cob.aastore(); - } + cob.aload(BUILDER_VAR); + loadArray(cob, CD_OPENS, sorted(opens), this::newOpens); cob.invokevirtual(CD_MODULE_BUILDER, "opens", MTD_OPENS_ARRAY) @@ -1307,24 +1287,22 @@ void opens(Set opens) { * or * Builder.newOpens(Set ms, String pn) * - * Set targets = new HashSet<>(); - * targets.add(t); - * : - * : - * - * Set mods = ... + * ms = open.modifiers() + * pn = open.source() + * targets = open.targets() * Builder.newOpens(mods, pn, targets); */ - void newOpens(Set ms, String pn, Set targets) { - dedupSetBuilder.loadOpensModifiers(cob, ms); - cob.loadConstant(pn); + void newOpens(CodeBuilder cb, Opens open) { + dedupSetBuilder.loadOpensModifiers(cb, open.modifiers()); + cb.loadConstant(open.source()); + var targets = open.targets(); if (!targets.isEmpty()) { - dedupSetBuilder.loadStringSet(cob, targets); - cob.invokestatic(CD_MODULE_BUILDER, + dedupSetBuilder.loadStringSet(cb, targets); + cb.invokestatic(CD_MODULE_BUILDER, "newOpens", MTD_OPENS_MODIFIER_SET_STRING_SET); } else { - cob.invokestatic(CD_MODULE_BUILDER, + cb.invokestatic(CD_MODULE_BUILDER, "newOpens", MTD_OPENS_MODIFIER_SET_STRING); } @@ -1349,16 +1327,8 @@ void uses(Set uses) { * */ void provides(Collection provides) { - cob.aload(BUILDER_VAR) - .loadConstant(provides.size()) - .anewarray(CD_PROVIDES); - int arrayIndex = 0; - for (Provides provide : sorted(provides)) { - cob.dup() // arrayref - .loadConstant(arrayIndex++); - newProvides(provide.service(), provide.providers()); - cob.aastore(); - } + cob.aload(BUILDER_VAR); + loadArray(cob, CD_PROVIDES, sorted(provides), this::newProvides); cob.invokevirtual(CD_MODULE_BUILDER, "provides", MTD_PROVIDES_ARRAY) @@ -1366,25 +1336,15 @@ void provides(Collection provides) { } /* - * Invoke Builder.newProvides(String service, Set providers) + * Invoke Builder.newProvides(String service, List providers) * - * Set providers = new HashSet<>(); - * providers.add(impl); - * : - * : + * service = provide.service() + * providers = List.of(new String[] { provide.providers() } * Builder.newProvides(service, providers); */ - void newProvides(String service, List providers) { - cob.loadConstant(service) - .loadConstant(providers.size()) - .anewarray(CD_String); - int arrayIndex = 0; - for (String provider : providers) { - cob.dup() // arrayref - .loadConstant(arrayIndex++) - .loadConstant(provider) - .aastore(); - } + void newProvides(CodeBuilder cb, Provides provide) { + cb.loadConstant(provide.service()); + loadArray(cb, CD_String, provide.providers(), CodeBuilder::loadConstant); cob.invokestatic(CD_List, "of", MTD_List_ObjectArray, @@ -1395,25 +1355,20 @@ void newProvides(String service, List providers) { } /* - * Invoke Builder.packages(String pn) + * Invoke Builder.packages(Set packages) + * with packages either from invoke provider method + * modulePackages() + * or construct inline with + * Set.of(packages) */ void packages(Set packages) { cob.aload(BUILDER_VAR); if (packages.size() > SET_SIZE_THRESHOLD) { var methodName = "module" + index + "Packages"; - addModuleHelpers(clb -> { - clb.withMethodBody( - methodName, - MethodTypeDesc.of(CD_Set), - ACC_STATIC, - cob -> { - genImmutableSet(cob, packages); - cob.areturn(); - }); - }); + addModuleHelpers(clb -> genImmutableSetProvider(clb, methodName, sorted(packages), CodeBuilder::loadConstant)); cob.invokestatic(classDesc, methodName, MethodTypeDesc.of(CD_Set)); } else { - genImmutableSet(cob, packages); + loadImmutableSet(cob, sorted(packages), CodeBuilder::loadConstant); } cob.invokevirtual(CD_MODULE_BUILDER, "packages", @@ -1652,6 +1607,18 @@ void loadRequiresModifiers(CodeBuilder cob, Set mods) { * Adding provider methods to the class. For those set used more than once, built * once and keep the reference for later access. * Return a list of snippet to be used in . + * + * The returned snippet would set up the set referenced more than once, + * + * static final Set[] dedupSetValues; + * + * static { + * dedupSetValues = new Set[countOfStoredValues]; + * dedupSetValues[0] = Set.of(elements); // elements no more than SET_SIZE_THRESHOLD + * dedupSetValues[1] = dedupProvider(); // set elements more than SET_SIZE_THRESHOLD + * ... + * dedupSetValues[countOfStoredValues - 1] = ... + * } */ Collection> buildConstants(ClassBuilder clb) { var index = 0; @@ -1675,25 +1642,23 @@ Collection> buildConstants(ClassBuilder clb) { } if (countOfStoredValues > 0) { + // The request cache slots each should have an initial value assert setValueBuilders.size() == countOfStoredValues; clb.withField(VALUES_ARRAY, CD_Set.arrayType(), ACC_STATIC | ACC_FINAL); // Allocate array before assign values setValueBuilders.addFirst(cob -> cob.loadConstant(countOfStoredValues) .anewarray(CD_Set) + .dup() .putstatic(owner, VALUES_ARRAY, CD_Set.arrayType())); } return setValueBuilders; } - void loadValuesArray(CodeBuilder cob) { - cob.getstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); - } - /* - * SetReference count references to the set, and use a CodeBuilder that - * generate bytecode to load an element onto the operand stack to generate bytecode - * to support loading the set onto operand stack. + * SetReference count references to the set, and use an element loader, which is + * a CodeBuilder that generate bytecode snippet to load an element onto the operand + * stack, to generate bytecode to support loading the set onto operand stack. * * When a set size is over SET_SIZE_THRESHOLD, a provider function is generated * to build the set rather than inline to avoid method size overflow. @@ -1705,15 +1670,18 @@ void loadValuesArray(CodeBuilder cob) { * load method can then be called to load the set onto the operand stack. */ class SetReference> implements Comparable> { - private final Set elements; + // sorted elements of the set to ensure same generated code + private final List elements; private final BiConsumer elementLoader; private int refCount; + // The index for this set value in the cache array private int index = -1; + // The provider method name, null if ths set is small enough for inline generation private String methodName; SetReference(Set elements, BiConsumer elementLoader) { - this.elements = elements; + this.elements = sorted(elements); this.elementLoader = elementLoader; } @@ -1721,12 +1689,18 @@ int increment() { return ++refCount; } - - // Load the set to the operand stack + // Load the set to the operand stack. + // When referenced more than once, the value is pre-built with static initialzer + // and is load from the cache array with + // dedupSetValues[index] + // Otherwise, built the set in place with either + // Set.of(elements) + // or invoke the generated provider method + // methodName() void load(CodeBuilder cob) { if (refCount > 1) { assert index >= 0; - loadValuesArray(cob); + cob.getstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); cob.loadConstant(index); cob.aaload(); } else { @@ -1734,16 +1708,25 @@ void load(CodeBuilder cob) { } } - // Build the set value and store the reference + // Build the set value and store the reference. + // Generate either + // dedupSetValues[index] = Set.of(elements); + // or + // dedupSetValues[index] = methodName(); void store(CodeBuilder cob) { assert index >= 0; - loadValuesArray(cob); - cob.loadConstant(index); + // array should be on top of the operands for this generate code in clinit + cob.dup() + .loadConstant(index); build(cob); cob.aastore(); } - // build the set and leave the reference at top of the operand stack + // Build the set and leave the reference at top of the operand stack. + // Generate either + // Set.of(elements) + // or invoke the provider method + // methodName() private void build(CodeBuilder cob) { if (methodName != null) { cob.invokestatic(owner, methodName, MethodTypeDesc.of(CD_Set)); @@ -1777,8 +1760,8 @@ public int compareTo(SetReference o) { return 0; } if (elements.size() == o.elements.size()) { - var a1 = sorted(elements); - var a2 = sorted(o.elements); + var a1 = elements; + var a2 = o.elements; for (int i = 0; i < elements.size(); i++) { var r = a1.get(i).compareTo(a2.get(i)); if (r != 0) { @@ -1792,51 +1775,6 @@ public int compareTo(SetReference o) { } } } - - static > BiConsumer getEnumLoader(ClassDesc enumClassDesc) { - return (cob, element) -> cob.getstatic(enumClassDesc, element.name(), enumClassDesc); - } - - static > void loadImmutableSet(CodeBuilder cob, - Set elements, - BiConsumer elementLoader) { - if (elements.size() <= 10) { - // call Set.of(e1, e2, ...) - for (T t : sorted(elements)) { - elementLoader.accept(cob, t); - } - var mtdArgs = new ClassDesc[elements.size()]; - Arrays.fill(mtdArgs, CD_Object); - cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, mtdArgs), true); - } else { - // call Set.of(E... elements) - cob.loadConstant(elements.size()) - .anewarray(CD_String); - int arrayIndex = 0; - for (T t : sorted(elements)) { - cob.dup() // arrayref - .loadConstant(arrayIndex); - elementLoader.accept(cob, t); // value - cob.aastore(); - arrayIndex++; - } - cob.invokestatic(CD_Set, "of", MTD_Set_ObjectArray, true); - } - } - - static > void genImmutableSetProvider(ClassBuilder clb, - String methodName, - Set elements, - BiConsumer elementLoader) { - clb.withMethodBody( - methodName, - MethodTypeDesc.of(CD_Set), - ACC_STATIC, - cob -> { - loadImmutableSet(cob, elements, elementLoader); - cob.areturn(); - }); - } } /** From 1e552d010f9c1952eb73d89716562ed567c84df3 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Sat, 26 Oct 2024 12:04:16 -0700 Subject: [PATCH 06/15] Loadable support with paging support. The limiting factor is now constant pool size --- .../jdk/tools/jlink/internal/Snippets.java | 497 +++++++++++++++--- .../internal/plugins/SystemModulesPlugin.java | 328 ++++++------ ...0Packages.java => JLink20000Packages.java} | 18 +- test/jdk/tools/jlink/SnippetsTest.java | 305 +++++++++++ 4 files changed, 896 insertions(+), 252 deletions(-) rename test/jdk/tools/jlink/{JLink3500Packages.java => JLink20000Packages.java} (88%) create mode 100644 test/jdk/tools/jlink/SnippetsTest.java diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java index 367a72b0cc6b0..c9571a9c11416 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -24,112 +24,467 @@ */ package jdk.tools.jlink.internal; +import java.lang.annotation.Target; import java.lang.classfile.ClassBuilder; +import static java.lang.classfile.ClassFile.ACC_PUBLIC; import static java.lang.classfile.ClassFile.ACC_STATIC; import java.lang.classfile.CodeBuilder; import java.lang.constant.ClassDesc; +import static java.lang.constant.ConstantDescs.CD_Integer; import static java.lang.constant.ConstantDescs.CD_Object; import static java.lang.constant.ConstantDescs.CD_Set; +import static java.lang.constant.ConstantDescs.CD_int; import java.lang.constant.MethodTypeDesc; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.function.BiConsumer; +import jdk.tools.jlink.internal.Snippets.ElementLoader; + public class Snippets { + // Tested page size of string array + public static final int STRING_PAGE_SIZE = 8000; + + public static final ElementLoader STRING_LOADER = ElementLoader.of(CodeBuilder::loadConstant); + public static final ElementLoader INTEGER_LOADER = (cob, value, index) -> { + // loadConstant will unbox + cob.loadConstant(value) + .invokestatic(ClassDesc.ofInternalName("java/lang/Integer"), "valueOf", MethodTypeDesc.of(CD_Integer, CD_int)); + }; + public static final ElementLoader LOADABLE_LOADER = (cob, loadable, index) -> loadable.load(cob); + + /** + * Describe a reference that can be load onto the operand stack. + * For example, an array of string can be described as a Loadable. + * The {@link load} method + */ + public sealed interface Loadable { + /** + * Generate the bytecode to load the Loadable onto the operand stack. + * @param cob The CodeBuilder to add the bytecode for loading + */ + void load(CodeBuilder cob); + + /** + * The type of the reference be loaded onto the operatnd stack. + */ + ClassDesc classDesc(); + + /** + * Generate fields or methods needed to support the load of the Loadable. + * @param clb The ClassBuilder to setup the helpers. + */ + default void setup(ClassBuilder clb) {}; + + /** + * Whether {@link setup} must be called to {@link load} properly. + */ + default boolean doesRequireSetup() { return false; } + } + + /** + * Generate a provider method for the {@code Loadable}. The provided + * Loadable should be ready for load. The caller is responsible to ensure + * the given Loadable had being setup properly. + * @param value The actuall {@code Loadable} to be wrapped into a method + * @param ownerClass The class of the generated method + * @param methodName The method name + * @param isStatic Should the generated method be static or public + * @throws IllegalArgumentException if the value is a {@code WrappedLoadable} + */ + public record WrappedLoadable(Loadable value, ClassDesc ownerClass, String methodName, boolean isStatic) implements Loadable { + public WrappedLoadable { + if (value instanceof WrappedLoadable) { + throw new IllegalArgumentException(); + } + } + + @Override + public void load(CodeBuilder cob) { + if (isStatic()) { + cob.invokestatic(ownerClass, methodName, methodType()); + } else { + cob.aload(0) + .invokevirtual(ownerClass, methodName, methodType()); + } + } + + @Override + public void setup(ClassBuilder clb) { + // TODO: decide whether we should call value.setup(clb) + // Prefer to have creator be responsible, given value + // is provided to constructor, it should be ready to use. + clb.withMethodBody( + methodName, + methodType(), + isStatic ? ACC_STATIC : ACC_PUBLIC, + cob -> { + value.load(cob); + cob.areturn(); + }); + } + + @Override + public ClassDesc classDesc() { + return value.classDesc(); + } + + @Override + public boolean doesRequireSetup() { + return true; + } + + /** + * Describe the method type of the generated provider method. + */ + public MethodTypeDesc methodType() { + return MethodTypeDesc.of(classDesc()); + } + } + + public record LoadableEnum(Enum o) implements Loadable { + @Override + public void load(CodeBuilder cob) { + cob.getstatic(classDesc(), o.name(), classDesc()); + } + + @Override + public ClassDesc classDesc() { + return o.getClass().describeConstable().get(); + } + } + + /** + * A function to load an element of type {@code T} onto the operand stack. + * @param cob The {@link CodeBuilder} to generate load code. + * @param element The element to be load. + * @param index The index of the element in the containing collection. + */ + public interface ElementLoader { + void load(CodeBuilder cob, T element, int index); + + static ElementLoader of(BiConsumer ignoreIndex) { + return (cob, element, _) -> { + ignoreIndex.accept(cob, element); + }; + } + + @SuppressWarnings("unchecked") + static ElementLoader selfLoader() { + return (ElementLoader) LOADABLE_LOADER; + } + } + /** * Return a snippet builder that loads an enum onto the operand stack using * the enum name static final field */ - public static > BiConsumer getEnumLoader(ClassDesc enumClassDesc) { - return (cob, element) -> cob.getstatic(enumClassDesc, element.name(), enumClassDesc); + public static > ElementLoader getEnumLoader(ClassDesc enumClassDesc) { + return (cob, element, _) -> cob.getstatic(enumClassDesc, element.name(), enumClassDesc); + } + + // Array supports + public sealed interface LoadableArray extends Loadable { + /** + * Factory method to create a LoadableArray. + * The bytecode generated varies based on the number of elements and can have supporting + * methods for pagination, helps to overcome the code size limitation. + * + * @param elementType The type of the array element + * @param elements The elements for the array + * @param elementLoader The loader function to load a single element onto operand stack to + * be stored at given index + * @param activatePagingThreshold Use pagination methods if the count of elements is larger + * than the given value + * @param ownerClassDesc The owner class for the paginattion methods + * @param methodNamePrefix The method name prefix. Generated method will have the name of + * this value appended with page number + * @param pageSize The count of elements per page + * + * @return A LoadableArray + */ + static LoadableArray of(ClassDesc elementType, + Collection elements, + ElementLoader elementLoader, + int activatePagingThreshold, + ClassDesc ownerClassDesc, + String methodNamePrefix, + int pageSize) { + if (elements.size() > activatePagingThreshold) { + return new PaginatedArray<>(elementType, elements, elementLoader, ownerClassDesc, methodNamePrefix, pageSize); + } else { + return new SimpleArray<>(elementType, elements, elementLoader); + } + } } /** - * Generate bytecode to create an array and load onto the operand stack. - * Effectively like following pseudo code: + * Base class for all LoadableArray implementation. + */ + private sealed static abstract class AbstractLoadableArray implements LoadableArray { + protected final ClassDesc elementType; + protected final Collection elements; + protected final ElementLoader elementLoader; + + public AbstractLoadableArray(ClassDesc elementType, Collection elements, ElementLoader elementLoader) { + this.elementType = elementType; + this.elements = elements; + this.elementLoader = elementLoader; + } + + @Override + public ClassDesc classDesc() { + return elementType.arrayType(); + } + + protected void fill(CodeBuilder cob, Iterable elements, int offset) { + for (T t : elements) { + cob.dup() // arrayref + .loadConstant(offset); + elementLoader.load(cob, t, offset); // value + cob.aastore(); + offset++; + } + } + } + + /** + * Generate bytecode to create an array and assign values inline. Effectively as * new T[] { elements } - * - * @param cob The code builder to add the snipper - * @param elementType The class descriptor of the element type T - * @param elements The elements to be in the array - * @param elementLoader A snippet generator to load an element T onto the operand stack. */ - public static void loadArray(CodeBuilder cob, - ClassDesc elementType, - Collection elements, - BiConsumer elementLoader) { - cob.loadConstant(elements.size()) - .anewarray(elementType); - int arrayIndex = 0; - for (T t : elements) { - cob.dup() // arrayref - .loadConstant(arrayIndex); - elementLoader.accept(cob, t); // value - cob.aastore(); - arrayIndex++; + public static final class SimpleArray extends AbstractLoadableArray { + public SimpleArray(ClassDesc elementType, T[] elements, ElementLoader elementLoader) { + this(elementType, Arrays.asList(elements), elementLoader); + } + + public SimpleArray(ClassDesc elementType, Collection elements, ElementLoader elementLoader) { + super(elementType, elements, elementLoader); + } + + @Override + public void load(CodeBuilder cob) { + cob.loadConstant(elements.size()) + .anewarray(elementType); + fill(cob, elements, 0); } } /** - * Generates bytecode to load a set onto the operand stack. - * Effectively like following pseudo code: - * Set.of(elements) - * @param cob The code builder to add the snippet - * @param elements The set to be created - * @param elementLoader Snippet generator to load an element onto the operand stack + * Generate bytecode for pagination methods, then create the array inline and invoke the first page method to assign + * values to the array. Each pagination method will assign value to the corresponding page and chain calling next + * page. + * {@code setup} must be called to generate the pagination methods in the owner class. Otherwise, {@code load} will + * lead to {@link java.lang.NoSuchMethodException} + * + * Effectively as + * methodNamePrefix0(new T[elements.size()]); + * + * where + * T[] methodNamePrefix0(T[] ar) { + * ar[0] = elements[0]; + * ar[1] = elements[1]; + * ... + * ar[pageSize-1] = elements[pageSize - 1]; + * methodNamePrefix1(ar); + * return ar; + * } + * and the last page will stop the chain and can be partial instead of full page size. */ - public static void loadImmutableSet(CodeBuilder cob, - Collection elements, - BiConsumer elementLoader) { - if (elements.size() <= 10) { - // call Set.of(e1, e2, ...) + public static final class PaginatedArray extends AbstractLoadableArray { + final int pageSize; + final ClassDesc ownerClassDesc; + final String methodNamePrefix; + final MethodTypeDesc MTD_PageHelper; + + public PaginatedArray(ClassDesc elementType, + T[] elements, + ElementLoader elementLoader, + ClassDesc ownerClassDesc, + String methodNamePrefix, + int pageSize) { + this(elementType, + Arrays.asList(elements), + elementLoader, + ownerClassDesc, + methodNamePrefix, + pageSize); + } + + public PaginatedArray(ClassDesc elementType, + Collection elements, + ElementLoader elementLoader, + ClassDesc ownerClassDesc, + String methodNamePrefix, + int pageSize) { + super(elementType, elements, elementLoader); + this.ownerClassDesc = ownerClassDesc; + this.methodNamePrefix = methodNamePrefix; + this.pageSize = pageSize; + MTD_PageHelper = MethodTypeDesc.of(classDesc(), classDesc()); + } + + @Override + public void load(CodeBuilder cob) { + // Invoke the first page, which will call next page until fulfilled + cob.loadConstant(elements.size()) + .anewarray(elementType) + .invokestatic(ownerClassDesc, methodNamePrefix + "0", MTD_PageHelper); + } + + @Override + public void setup(ClassBuilder clb) { + var pages = paginate(elements, pageSize); + + assert(pages.size() == pageCount()); + + var lastPageNo = pages.size() - 1; + for (int pageNo = 0; pageNo <= lastPageNo; pageNo++) { + genFillPageHelper(clb, pages.get(pageNo), pageNo, pageNo < lastPageNo); + } + } + + @Override + public boolean doesRequireSetup() { return true; } + + // each helper function is T[] methodNamePrefix{pageNo}(T[]) + // fill the page portion and chain calling to fill next page + private void genFillPageHelper(ClassBuilder clb, Collection pageElements, int pageNo, boolean hasNextPage) { + var offset = pageSize * pageNo; + clb.withMethodBody( + methodNamePrefix + pageNo, + MTD_PageHelper, + ACC_STATIC, + mcob -> { + mcob.aload(0); // arrayref + fill(mcob, pageElements, offset); + if (hasNextPage) { + mcob.invokestatic( + ownerClassDesc, + methodNamePrefix + (pageNo + 1), + MTD_PageHelper); + } + mcob.areturn(); + }); + } + + public boolean isLastPagePartial() { + return (elements.size() % pageSize) != 0; + } + + public int pageCount() { + var pages = elements.size() / pageSize; + return isLastPagePartial() ? pages + 1 : pages; + } + } + + // Set support + public sealed interface LoadableSet extends Loadable { + /** + * Factory method for LoadableSet without using pagination methods. + */ + static LoadableSet of(Collection elements, ElementLoader loader) { + // Set::of implementation optimization with 2 elements + if (elements.size() <= 2) { + return new TinySet<>(elements, loader); + } else { + return new ArrayAsSet<>(new SimpleArray<>(CD_Object, elements, loader)); + } + } + + /** + * Factory method for LoadableSet pagination methods when element count is larger than + * given threshold. + */ + static LoadableSet of(Collection elements, + ElementLoader loader, + int activatePagingThreshold, + ClassDesc ownerClassDesc, + String methodNamePrefix, + int pageSize) { + if (elements.size() > activatePagingThreshold) { + return new ArrayAsSet<>(LoadableArray.of( + CD_Object, + elements, + loader, + activatePagingThreshold, + ownerClassDesc, + methodNamePrefix, + pageSize)); + } else { + return LoadableSet.of(elements, loader); + } + } + + @Override + default ClassDesc classDesc() { + return CD_Set; + } + } + + private static final class TinySet implements LoadableSet { + Collection elements; + ElementLoader loader; + + TinySet(Collection elements, ElementLoader loader) { + // The Set::of API supports up to 10 elements + if (elements.size() > 10) { + throw new IllegalArgumentException(); + } + this.elements = elements; + this.loader = loader; + } + + @Override + public void load(CodeBuilder cob) { + var index = 0; for (T t : elements) { - elementLoader.accept(cob, t); + loader.load(cob, t, index++); } var mtdArgs = new ClassDesc[elements.size()]; Arrays.fill(mtdArgs, CD_Object); cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, mtdArgs), true); - } else { - // call Set.of(E... elements) - loadArray(cob, CD_Object, elements, elementLoader); - cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, CD_Object.arrayType()), true); } } - // Generate a method with pseudo code looks like this - // static Set methodName() { - // return Set.of(elements); - // } - public static void genImmutableSetProvider(ClassBuilder clb, - String methodName, - Collection elements, - BiConsumer elementLoader) { - clb.withMethodBody( - methodName, - MethodTypeDesc.of(CD_Set), - ACC_STATIC, - cob -> { - loadImmutableSet(cob, elements, elementLoader); - cob.areturn(); - }); + private static final class ArrayAsSet implements LoadableSet { + final LoadableArray elements; + + ArrayAsSet(LoadableArray elements) { + this.elements = elements; + } + + @Override + public void load(CodeBuilder cob) { + elements.load(cob); + cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, CD_Object.arrayType()), true); + } + + @Override + public boolean doesRequireSetup() { + return elements.doesRequireSetup(); + } + + @Override + public void setup(ClassBuilder clb) { + elements.setup(clb); + } } - // Generate a method with pseudo code looks like this - // static T[] methodName() { - // return new T[] { elements }; - // } - public static void genArrayProvider(ClassBuilder clb, - String methodName, - ClassDesc elementType, - Collection elements, - BiConsumer elementLoader) { - clb.withMethodBody( - methodName, - MethodTypeDesc.of(elementType.arrayType()), - ACC_STATIC, - cob -> { - loadArray(cob, elementType, elements, elementLoader); - cob.areturn(); - }); + // utilities + private static ArrayList> paginate(Iterable elements, int pageSize) { + ArrayList> pages = new ArrayList<>(pageSize); + ArrayList currentPage = null; + var index = 0; + for (T element: elements) { + if (index % pageSize == 0) { + currentPage = new ArrayList<>(); + pages.add(currentPage); + } + currentPage.add(element); + index++; + } + + return pages; } } \ No newline at end of file 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 11e0337f7454a..b09622269072d 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 @@ -87,11 +87,8 @@ import static java.lang.classfile.ClassFile.*; -import static jdk.tools.jlink.internal.Snippets.genArrayProvider; -import static jdk.tools.jlink.internal.Snippets.genImmutableSetProvider; -import static jdk.tools.jlink.internal.Snippets.getEnumLoader; -import static jdk.tools.jlink.internal.Snippets.loadArray; -import static jdk.tools.jlink.internal.Snippets.loadImmutableSet; +import static jdk.tools.jlink.internal.Snippets.*; + /** * Jlink plugin to reconstitute module descriptors and other attributes for system @@ -585,19 +582,19 @@ public SystemModulesClassGenerator(String className, */ private void dedups(ModuleDescriptor md) { // exports - for (Exports e : md.exports()) { + for (Exports e : sorted(md.exports())) { dedupSetBuilder.stringSet(e.targets()); dedupSetBuilder.exportsModifiers(e.modifiers()); } // opens - for (Opens opens : md.opens()) { + for (Opens opens : sorted(md.opens())) { dedupSetBuilder.stringSet(opens.targets()); dedupSetBuilder.opensModifiers(opens.modifiers()); } // requires - for (Requires r : md.requires()) { + for (Requires r : sorted(md.requires())) { dedupSetBuilder.requiresModifiers(r.modifiers()); } @@ -647,8 +644,10 @@ public byte[] genClassBytes(Configuration cf) { }); } - private void addModuleHelpers(Consumer amendment) { - amendments.add(amendment); + private void setupLoadable(Loadable loadable) { + if (loadable.doesRequireSetup()) { + amendments.add(loadable::setup); + } } /** @@ -674,9 +673,8 @@ private void genConstants(ClassBuilder clb) { MTD_void, ACC_STATIC, cob -> { - clinitSnippets.forEach(snippet -> snippet.accept(cob)); - cob.pop() - .return_(); + clinitSnippets.get().accept(cob); + cob.return_(); }); } } @@ -1019,7 +1017,8 @@ private void generate(ClassBuilder clb, * Generate code to generate an immutable set. */ private void genImmutableSet(CodeBuilder cob, Set set) { - loadImmutableSet(cob, sorted(set), CodeBuilder::loadConstant); + var loadableSet = LoadableSet.of(sorted(set), STRING_LOADER); + loadableSet.load(cob); } class ModuleDescriptorBuilder { @@ -1066,7 +1065,7 @@ class ModuleDescriptorBuilder { static final MethodTypeDesc MTD_ModuleDescriptor_int = MethodTypeDesc.of(CD_MODULE_DESCRIPTOR, CD_int); static final MethodTypeDesc MTD_List_ObjectArray = MethodTypeDesc.of(CD_List, CD_Object.arrayType()); - static final int SET_SIZE_THRESHOLD = 512; // An arbitrary number as this likely generate minimum ~4K code + static final int PAGING_THRESHOLD = 512; // An arbitrary number as this likely generate minimum ~4K code final CodeBuilder cob; final ModuleDescriptor md; @@ -1216,28 +1215,26 @@ void newRequires(Set mods, String name, String compiledVersio * */ void exports(Set exports) { + var exportArray = LoadableArray.of( + CD_EXPORTS, + sorted(exports), + this::loadExports, + PAGING_THRESHOLD, + classDesc, + "module" + index + "Exports", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(exportArray); + cob.aload(BUILDER_VAR); - loadExportsArray(exports); + exportArray.load(cob); cob.invokevirtual(CD_MODULE_BUILDER, "exports", MTD_EXPORTS_ARRAY) .pop(); } - void loadExportsArray(Set exports) { - if (exports.size() > SET_SIZE_THRESHOLD) { - String methodName = "module" + index + "Exports"; - addModuleHelpers(clb -> genArrayProvider(clb, - methodName, - CD_EXPORTS, - sorted(exports), - this::loadExports)); - cob.invokestatic(classDesc, methodName, MethodTypeDesc.of(CD_EXPORTS.arrayType())); - } else { - loadArray(cob, CD_EXPORTS, sorted(exports), this::loadExports); - } - } - /* * Invoke * Builder.newExports(Set ms, String pn, @@ -1249,7 +1246,7 @@ void loadExportsArray(Set exports) { * pn = export.source() * targets = export.targets() */ - void loadExports(CodeBuilder cb, Exports export) { + void loadExports(CodeBuilder cb, Exports export, int unused) { dedupSetBuilder.loadExportsModifiers(cb, export.modifiers()); cb.loadConstant(export.source()); var targets = export.targets(); @@ -1272,8 +1269,20 @@ void loadExports(CodeBuilder cb, Exports export) { * Builder.opens(Opens[]) */ void opens(Set opens) { + var opensArray = LoadableArray.of( + CD_OPENS, + sorted(opens), + this::newOpens, + PAGING_THRESHOLD, + classDesc, + "module" + index + "Opens", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(opensArray); + cob.aload(BUILDER_VAR); - loadArray(cob, CD_OPENS, sorted(opens), this::newOpens); + opensArray.load(cob); cob.invokevirtual(CD_MODULE_BUILDER, "opens", MTD_OPENS_ARRAY) @@ -1292,7 +1301,7 @@ void opens(Set opens) { * targets = open.targets() * Builder.newOpens(mods, pn, targets); */ - void newOpens(CodeBuilder cb, Opens open) { + void newOpens(CodeBuilder cb, Opens open, int unused) { dedupSetBuilder.loadOpensModifiers(cb, open.modifiers()); cb.loadConstant(open.source()); var targets = open.targets(); @@ -1327,8 +1336,20 @@ void uses(Set uses) { * */ void provides(Collection provides) { + var providesArray = LoadableArray.of( + CD_PROVIDES, + sorted(provides), + this::newProvides, + PAGING_THRESHOLD, + classDesc, + "module" + index + "Provides", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(providesArray); + cob.aload(BUILDER_VAR); - loadArray(cob, CD_PROVIDES, sorted(provides), this::newProvides); + providesArray.load(cob); cob.invokevirtual(CD_MODULE_BUILDER, "provides", MTD_PROVIDES_ARRAY) @@ -1342,10 +1363,22 @@ void provides(Collection provides) { * providers = List.of(new String[] { provide.providers() } * Builder.newProvides(service, providers); */ - void newProvides(CodeBuilder cb, Provides provide) { + void newProvides(CodeBuilder cb, Provides provide, int offset) { + var providersArray = LoadableArray.of( + CD_String, + provide.providers(), + STRING_LOADER, + PAGING_THRESHOLD, + classDesc, + "module" + index + "Provider" + offset, + STRING_PAGE_SIZE); + + + setupLoadable(providersArray); + cb.loadConstant(provide.service()); - loadArray(cb, CD_String, provide.providers(), CodeBuilder::loadConstant); - cob.invokestatic(CD_List, + providersArray.load(cb); + cb.invokestatic(CD_List, "of", MTD_List_ObjectArray, true) @@ -1362,14 +1395,18 @@ void newProvides(CodeBuilder cb, Provides provide) { * Set.of(packages) */ void packages(Set packages) { + var packagesArray = LoadableSet.of( + sorted(packages), + STRING_LOADER, + PAGING_THRESHOLD, + classDesc, + "module" + index + "Packages", + STRING_PAGE_SIZE); + + setupLoadable(packagesArray); + cob.aload(BUILDER_VAR); - if (packages.size() > SET_SIZE_THRESHOLD) { - var methodName = "module" + index + "Packages"; - addModuleHelpers(clb -> genImmutableSetProvider(clb, methodName, sorted(packages), CodeBuilder::loadConstant)); - cob.invokestatic(classDesc, methodName, MethodTypeDesc.of(CD_Set)); - } else { - loadImmutableSet(cob, sorted(packages), CodeBuilder::loadConstant); - } + packagesArray.load(cob); cob.invokevirtual(CD_MODULE_BUILDER, "packages", MTD_SET) @@ -1399,15 +1436,6 @@ void version(Version v) { MTD_STRING) .pop(); } - - void invokeBuilderMethod(String methodName, String value) { - cob.aload(BUILDER_VAR) - .loadConstant(value) - .invokevirtual(CD_MODULE_BUILDER, - methodName, - MTD_STRING) - .pop(); - } } class ModuleHashesBuilder { @@ -1508,34 +1536,43 @@ void hashForModule(String name, byte[] hash) { static class DedupSetBuilder { // map Set to a specialized builder to allow them to be // deduplicated as they are requested - final Map, SetReference> stringSets = new HashMap<>(); + final Map, SetReference> stringSets = new HashMap<>(); // map Set to a specialized builder to allow them to be // deduplicated as they are requested - final Map, SetReference> + final Map, SetReference> requiresModifiersSets = new HashMap<>(); // map Set to a specialized builder to allow them to be // deduplicated as they are requested - final Map, SetReference> + final Map, SetReference> exportsModifiersSets = new HashMap<>(); // map Set to a specialized builder to allow them to be // deduplicated as they are requested - final Map, SetReference> + final Map, SetReference> opensModifiersSets = new HashMap<>(); private static final String VALUES_ARRAY = "dedupSetValues"; final ClassDesc owner; - int countOfStoredValues = 0; + private final ArrayList values = new ArrayList<>(); DedupSetBuilder(ClassDesc owner) { this.owner = owner; } - int requestValueStorage() { - return countOfStoredValues++; + > SetReference createLoadableSet(Set elements, ElementLoader elementLoader) { + var loadableSet = LoadableSet.of(sorted(elements), + elementLoader, + ModuleDescriptorBuilder.PAGING_THRESHOLD, + owner, + "dedupSet" + values.size(), + // Safe for String and Enum within 64K + 3000); + var ref = new SetReference(loadableSet); + values.add(ref); + return ref; } /* @@ -1543,7 +1580,7 @@ int requestValueStorage() { */ void stringSet(Set strings) { stringSets.computeIfAbsent(strings, - s -> new SetReference<>(s, CodeBuilder::loadConstant) + s -> createLoadableSet(s, STRING_LOADER) ).increment(); } @@ -1551,8 +1588,8 @@ void stringSet(Set strings) { * Add the given set of Exports.Modifiers */ void exportsModifiers(Set mods) { - exportsModifiersSets.computeIfAbsent(mods, s -> - new SetReference<>(s, getEnumLoader(CD_EXPORTS_MODIFIER)) + exportsModifiersSets.computeIfAbsent(mods, + s -> createLoadableSet(s, getEnumLoader(CD_EXPORTS_MODIFIER)) ).increment(); } @@ -1560,8 +1597,8 @@ void exportsModifiers(Set mods) { * Add the given set of Opens.Modifiers */ void opensModifiers(Set mods) { - opensModifiersSets.computeIfAbsent(mods, s -> - new SetReference<>(s, getEnumLoader(CD_OPENS_MODIFIER)) + opensModifiersSets.computeIfAbsent(mods, + s -> createLoadableSet(s, getEnumLoader(CD_OPENS_MODIFIER)) ).increment(); } @@ -1569,8 +1606,8 @@ void opensModifiers(Set mods) { * Add the given set of Requires.Modifiers */ void requiresModifiers(Set mods) { - requiresModifiersSets.computeIfAbsent(mods, s -> - new SetReference<>(s, getEnumLoader(CD_REQUIRES_MODIFIER)) + requiresModifiersSets.computeIfAbsent(mods, + s -> createLoadableSet(s, getEnumLoader(CD_REQUIRES_MODIFIER)) ).increment(); } @@ -1620,39 +1657,44 @@ void loadRequiresModifiers(CodeBuilder cob, Set mods) { * dedupSetValues[countOfStoredValues - 1] = ... * } */ - Collection> buildConstants(ClassBuilder clb) { - var index = 0; - ArrayList> setValueBuilders = new ArrayList<>(); - // The SetReferences need to be sorted to reproduce same result. - for (var ref : sorted(stringSets.values())) { - index++; - ref.generateConstant(clb, "dedupStringSet" + index).ifPresent(setValueBuilders::add); - } - for (var ref: sorted(opensModifiersSets.values())) { - index++; - ref.generateConstant(clb, "dedupOpensSet" + index).ifPresent(setValueBuilders::add); - } - for (var ref: sorted(exportsModifiersSets.values())) { - index++; - ref.generateConstant(clb, "dedupExportsSet" + index).ifPresent(setValueBuilders::add); - } - for (var ref: sorted(requiresModifiersSets.values())) { - index++; - ref.generateConstant(clb, "dedupRequiresSet" + index).ifPresent(setValueBuilders::add); + Optional> buildConstants(ClassBuilder clb) { + var staticCache = new ArrayList(); + + for (var set: values) { + set.loadableSet().setup(clb); + if (set.refCount() > 1) { + staticCache.add(set); + } } - if (countOfStoredValues > 0) { - // The request cache slots each should have an initial value - assert setValueBuilders.size() == countOfStoredValues; - clb.withField(VALUES_ARRAY, CD_Set.arrayType(), ACC_STATIC | ACC_FINAL); - // Allocate array before assign values - setValueBuilders.addFirst(cob -> - cob.loadConstant(countOfStoredValues) - .anewarray(CD_Set) - .dup() - .putstatic(owner, VALUES_ARRAY, CD_Set.arrayType())); + if (staticCache.isEmpty()) { + return Optional.empty(); } - return setValueBuilders; + + // This is called when the value is build for the cache + // At that time, a slot in cache is assigned + // The loader is called when building the static initializer + // We need to ensure that happens before we access SetReference::load + ElementLoader cacheLoader = (cob, setRef, index) -> { + setRef.assignTo(index); + setRef.loadableSet().load(cob); + }; + + var loadableArray = LoadableArray.of( + CD_Set, + staticCache, + cacheLoader, + ModuleDescriptorBuilder.PAGING_THRESHOLD, + owner, + VALUES_ARRAY, + 2000); + + loadableArray.setup(clb); + clb.withField(VALUES_ARRAY, CD_Set.arrayType(), ACC_STATIC | ACC_FINAL); + return Optional.of(cob -> { + loadableArray.load(cob); + cob.putstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); + }); } /* @@ -1669,26 +1711,34 @@ Collection> buildConstants(ClassBuilder clb) { * generateConstant method should be called to setup the provider methods and cache array. * load method can then be called to load the set onto the operand stack. */ - class SetReference> implements Comparable> { + class SetReference { // sorted elements of the set to ensure same generated code - private final List elements; - private final BiConsumer elementLoader; + private final LoadableSet loadableSet; private int refCount; // The index for this set value in the cache array private int index = -1; - // The provider method name, null if ths set is small enough for inline generation - private String methodName; - SetReference(Set elements, BiConsumer elementLoader) { - this.elements = sorted(elements); - this.elementLoader = elementLoader; + SetReference(LoadableSet set) { + this.loadableSet = set; } int increment() { return ++refCount; } + int refCount() { + return refCount; + } + + LoadableSet loadableSet() { + return loadableSet; + } + + void assignTo(int index) { + this.index = index; + } + // Load the set to the operand stack. // When referenced more than once, the value is pre-built with static initialzer // and is load from the cache array with @@ -1704,73 +1754,7 @@ void load(CodeBuilder cob) { cob.loadConstant(index); cob.aaload(); } else { - build(cob); - } - } - - // Build the set value and store the reference. - // Generate either - // dedupSetValues[index] = Set.of(elements); - // or - // dedupSetValues[index] = methodName(); - void store(CodeBuilder cob) { - assert index >= 0; - // array should be on top of the operands for this generate code in clinit - cob.dup() - .loadConstant(index); - build(cob); - cob.aastore(); - } - - // Build the set and leave the reference at top of the operand stack. - // Generate either - // Set.of(elements) - // or invoke the provider method - // methodName() - private void build(CodeBuilder cob) { - if (methodName != null) { - cob.invokestatic(owner, methodName, MethodTypeDesc.of(CD_Set)); - } else { - loadImmutableSet(cob, elements, elementLoader); - } - } - - /** - * Generate provider method if the set size is over threshold to avoid overload - * bytecode limitation per method. - * Return a snippet builder that generates code to store the reference of the set value. - */ - Optional> generateConstant(ClassBuilder clb, String name) { - if (elements.size() > ModuleDescriptorBuilder.SET_SIZE_THRESHOLD) { - methodName = name + "Provider"; - genImmutableSetProvider(clb, methodName, elements, elementLoader); - } - - if (refCount <= 1) { - return Optional.empty(); - } else { - index = requestValueStorage(); - return Optional.of(this::store); - } - } - - @Override - public int compareTo(SetReference o) { - if (o == this) { - return 0; - } - if (elements.size() == o.elements.size()) { - var a1 = elements; - var a2 = o.elements; - for (int i = 0; i < elements.size(); i++) { - var r = a1.get(i).compareTo(a2.get(i)); - if (r != 0) { - return r; - } - } - return 0; - } else { - return elements.size() - o.elements.size(); + loadableSet.load(cob); } } } diff --git a/test/jdk/tools/jlink/JLink3500Packages.java b/test/jdk/tools/jlink/JLink20000Packages.java similarity index 88% rename from test/jdk/tools/jlink/JLink3500Packages.java rename to test/jdk/tools/jlink/JLink20000Packages.java index 917ba439e001a..bdb7a42540606 100644 --- a/test/jdk/tools/jlink/JLink3500Packages.java +++ b/test/jdk/tools/jlink/JLink20000Packages.java @@ -32,7 +32,7 @@ /* * @test - * @summary Make sure that ~3500 packages in a uber jar can be linked using jlink. Depends on the + * @summary Make sure that ~20000 packages in a uber jar can be linked using jlink. Depends on the * packages, this is almost hit the 64K limitation as each plain export could take * ~17 bytecodes. * @bug 8321413 @@ -45,9 +45,9 @@ * jdk.jlink/jdk.tools.jimage * jdk.compiler * @build tests.* - * @run main/othervm -Xmx1g -Xlog:init=debug -XX:+UnlockDiagnosticVMOptions -XX:+BytecodeVerificationLocal JLink3500Packages + * @run main/othervm -Xmx1g -Xlog:init=debug -XX:+UnlockDiagnosticVMOptions -XX:+BytecodeVerificationLocal JLink20000Packages */ -public class JLink3500Packages { +public class JLink20000Packages { private static final ToolProvider JAVAC_TOOL = ToolProvider.findFirst("javac") .orElseThrow(() -> new RuntimeException("javac tool not found")); @@ -66,7 +66,7 @@ public static void main(String[] args) throws Exception { StringJoiner mainModuleInfoContent = new StringJoiner(";\n exports ", "module bug8321413x {\n exports ", ";\n}"); - for (int i = 0; i < 3500; i++) { + for (int i = 0; i < 20000; i++) { String packageName = "p" + i; String className = "C" + i; @@ -87,12 +87,12 @@ public static void main(String[] args) throws Exception { Path mainClassDir = mainModulePath.resolve("testpackage"); Files.createDirectories(mainClassDir); - Files.writeString(mainClassDir.resolve("JLink3500PackagesTest.java"), """ + Files.writeString(mainClassDir.resolve("JLink20000PackagesTest.java"), """ package testpackage; - public class JLink3500PackagesTest { + public class JLink20000PackagesTest { public static void main(String[] args) throws Exception { - System.out.println("JLink3500PackagesTest started."); + System.out.println("JLink20000PackagesTest started."); } } """); @@ -117,12 +117,12 @@ public static void main(String[] args) throws Exception { ProcessBuilder processBuilder = new ProcessBuilder(bin.toString(), "-XX:+UnlockDiagnosticVMOptions", "-XX:+BytecodeVerificationLocal", - "-m", "bug8321413x/testpackage.JLink3500PackagesTest"); + "-m", "bug8321413x/testpackage.JLink20000PackagesTest"); processBuilder.inheritIO(); processBuilder.directory(binDir.toFile()); Process process = processBuilder.start(); int exitCode = process.waitFor(); if (exitCode != 0) - throw new AssertionError("JLink3500PackagesTest failed to launch"); + throw new AssertionError("JLink20000PackagesTest failed to launch"); } } diff --git a/test/jdk/tools/jlink/SnippetsTest.java b/test/jdk/tools/jlink/SnippetsTest.java new file mode 100644 index 0000000000000..e42c8d165efe7 --- /dev/null +++ b/test/jdk/tools/jlink/SnippetsTest.java @@ -0,0 +1,305 @@ +/* + * Copyright (c) 2024, 2024, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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.io.IOException; +import java.lang.classfile.ClassFile; +import static java.lang.classfile.ClassFile.ACC_PUBLIC; +import java.lang.constant.ClassDesc; +import static java.lang.constant.ConstantDescs.CD_Integer; +import static java.lang.constant.ConstantDescs.CD_Object; +import static java.lang.constant.ConstantDescs.CD_String; +import static java.lang.constant.ConstantDescs.INIT_NAME; +import static java.lang.constant.ConstantDescs.MTD_void; +import java.lang.constant.MethodTypeDesc; +import static java.lang.invoke.MethodHandles.lookup; +import java.lang.invoke.MethodType; +import java.lang.module.ModuleDescriptor; +import java.lang.reflect.AccessFlag; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.*; +import java.util.function.Supplier; +import java.util.stream.IntStream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import static org.junit.jupiter.api.Assertions.*; + +import jdk.tools.jlink.internal.Snippets.*; +import static jdk.tools.jlink.internal.Snippets.*; +import jdk.tools.jlink.internal.Snippets.ElementLoader; + +/* + * @test + * @summary Test snippets generation for array and set. + * @bug 8321413 + * @enablePreview + * @modules jdk.jlink/jdk.tools.jlink.internal + * @run junit SnippetsTest + */ +public class SnippetsTest { + private static final boolean WRITE_CLASS_FILE = Boolean.parseBoolean(System.getProperty("DumpArraySnippetsTestClasses", "true")); + + @ParameterizedTest + @ValueSource(ints = { 10, 75, 90, 120, 200, 399, 400, 401}) + void testLoad400StringsArray(int pageSize) { + testPaginatedArray(400, pageSize); + } + + @Test + void testStringArrayLimitsWithPagination() { + // Each string takes 2 constant pool slot, one for String, another for Utf8 + testPaginatedArray(31_000, 8000); + try { + testPaginatedArray(32_000, 8000); + } catch (IllegalArgumentException iae) { + // expected constant pool explode + } + } + + @Test + void testStringArrayLimitsWithoutPagination() { + // each string array assignment takes ~8 bytes + testSimpleArray(8200); + try { + testSimpleArray(8300); + fail(); + } catch (IllegalArgumentException iae) { + // expected code size explode + } + } + + @Test + void testEnumLoader() { + ClassDesc CD_ACCESSFLAG = AccessFlag.class.describeConstable().get(); + var expected = AccessFlag.values(); + var loadable = new SimpleArray(CD_ACCESSFLAG, expected, getEnumLoader(CD_ACCESSFLAG)); + Supplier supplier = generateSupplier("TestEnumLoader", loadable); + assertArrayEquals(expected, supplier.get()); + } + + @ParameterizedTest + @ValueSource(booleans = { true, false }) + void testWrapperLoadable(boolean isStatic) throws NoSuchMethodException { + var expected = IntStream.range(0, 1234) + .mapToObj(i -> "WrapperTestString" + i) + .toList(); + var className = "WrapperLoadableTest" + (isStatic ? "Static" : "Public"); + ClassDesc testClassDesc = ClassDesc.of(className); + + var loadable = new PaginatedArray<>( + CD_String, expected, STRING_LOADER, testClassDesc, "page", 100); + // 1234 with 10 per page, should have 13 pages with last page 34 elements + assertEquals(13, loadable.pageCount()); + assertTrue(loadable.isLastPagePartial()); + + var wrapped = new WrappedLoadable(loadable, testClassDesc, "wrapper", isStatic); + Supplier supplier = generateSupplier(className, wrapped, loadable); + verifyPaginationMethods(supplier.getClass(), String.class, "page", 13); + assertArrayEquals(expected.toArray(), supplier.get()); + + // check wrapper function + var methodType = MethodType.methodType(String[].class); + try { + lookup().findStatic(supplier.getClass(), wrapped.methodName(), methodType); + } catch (IllegalAccessException ex) { + assertFalse(isStatic); + } + try { + lookup().findVirtual(supplier.getClass(), wrapped.methodName(), methodType); + } catch (IllegalAccessException ex) { + assertTrue(isStatic); + } + } + + @Test + void testLoadableEnum() { + Enum[] enums = { + AccessFlag.FINAL, + ModuleDescriptor.Requires.Modifier.MANDATED, + ModuleDescriptor.Opens.Modifier.SYNTHETIC, + ModuleDescriptor.Requires.Modifier.TRANSITIVE + }; + + var loadable = new SimpleArray( + Enum.class.describeConstable().get(), + Arrays.stream(enums).map(LoadableEnum::new).toList(), + ElementLoader.selfLoader()); + + Supplier[]> supplier = generateSupplier("LoadableEnumTest", loadable); + assertArrayEquals(enums, supplier.get()); + } + + @Test + void testLoadableArrayOf() { + Integer[] expected = IntStream.range(0, 200) + .boxed() + .toArray(Integer[]::new); + var className = "LoadableArrayOf200Paged"; + var loadable = LoadableArray.of(CD_Integer, + Arrays.asList(expected), + INTEGER_LOADER, + expected.length - 1, + ClassDesc.of(className), + "page", + 100); + assertTrue(loadable instanceof PaginatedArray); + + Supplier supplier = generateSupplier(className, loadable); + verifyPaginationMethods(supplier.getClass(), Integer.class, "page", 2); + assertArrayEquals(expected, supplier.get()); + + loadable = LoadableArray.of( + CD_Integer, + Arrays.asList(expected), + INTEGER_LOADER, + expected.length, + ClassDesc.of("LoadableArrayOf200NotPaged"), + "page", + 100); + assertTrue(loadable instanceof SimpleArray); + + // SimpleArray generate bytecode inline, so can be generated in any class + supplier = generateSupplier("TestLoadableArrayFactory", loadable); + assertArrayEquals(expected, supplier.get()); + } + + @Test + void testLoadableSetOf() { + String[] data = IntStream.range(0, 100) + .mapToObj(i -> "SetData" + i) + .toArray(String[]::new); + + var tiny = Set.of(data[0], data[1], data[2]); + var all = Set.of(data); + + Supplier> supplier = generateSupplier("TinySetTest", LoadableSet.of(tiny, STRING_LOADER)); + // Set does not guarantee ordering, so not assertIterableEquals + assertEquals(tiny, supplier.get()); + + supplier = generateSupplier("AllSetTestNoPage", LoadableSet.of(all, STRING_LOADER)); + assertEquals(all, supplier.get()); + + var className = "AllSetTestPageNotActivated"; + var methodNamePrefix = "page"; + var loadable = LoadableSet.of(all, STRING_LOADER, all.size(), + ClassDesc.of(className), methodNamePrefix, 10); + supplier = generateSupplier(className, loadable); + assertEquals(all, supplier.get()); + + className = "AllSetTestPageSize20"; + loadable = LoadableSet.of(all, STRING_LOADER, all.size() - 1, + ClassDesc.of(className), methodNamePrefix, 20); + supplier = generateSupplier(className, loadable); + // Set erased element type and use Object as element type + verifyPaginationMethods(supplier.getClass(), Object.class, methodNamePrefix, 5); + assertEquals(all, supplier.get()); + } + + void testPaginatedArray(int elementCount, int pageSize) { + String[] expected = IntStream.range(0, elementCount) + .mapToObj(i -> "Package" + i) + .toArray(String[]::new); + var className = String.format("SnippetArrayProviderTest%dPagedBy%d", elementCount, pageSize); + ClassDesc testClassDesc = ClassDesc.of(className); + var loadable = new PaginatedArray<>(CD_String, expected, STRING_LOADER, + testClassDesc, "ArrayPage", pageSize); + + Supplier supplier = generateSupplier(className, loadable); + verifyPaginationMethods(supplier.getClass(), String.class, "ArrayPage", loadable.pageCount()); + assertEquals((elementCount % pageSize) != 0, loadable.isLastPagePartial()); + assertArrayEquals(expected, supplier.get()); + } + + void testSimpleArray(int elementCount) { + String[] expected = IntStream.range(0, elementCount) + .mapToObj(i -> "NoPage" + i) + .toArray(String[]::new); + String className = "SnippetArrayProviderTest" + elementCount; + var array = new SimpleArray<>(CD_String, Arrays.asList(expected), STRING_LOADER); + + Supplier supplier = generateSupplier(className, array); + assertArrayEquals(expected, supplier.get()); + } + + Supplier generateSupplier(String className, Loadable loadable, Loadable... extra) { + var testClassDesc = ClassDesc.of(className); + byte[] classBytes = generateSupplierClass(testClassDesc, loadable, extra); + try { + writeClassFile(className, classBytes); + var testClass = lookup().defineClass(classBytes); + lookup().findVirtual(testClass, "get", MethodType.methodType(Object.class)); + return (Supplier) testClass.getDeclaredConstructor().newInstance(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + void verifyPaginationMethods(Class testClass, Class elementType, String methodNamePrefix, int pageCount) { + for (int i = 0; i < pageCount; i++) { + try { + lookup().findStatic(testClass, methodNamePrefix + i, + MethodType.methodType(elementType.arrayType(), elementType.arrayType())); + } catch (Exception ex) { + throw new RuntimeException(ex); + } + } + } + + byte[] generateSupplierClass(ClassDesc testClassDesc, Loadable loadable, Loadable... extra) { + return ClassFile.of().build(testClassDesc, + clb -> { + clb.withSuperclass(CD_Object); + clb.withInterfaceSymbols(ClassDesc.ofInternalName("java/util/function/Supplier")); + clb.withMethodBody(INIT_NAME, MTD_void, ACC_PUBLIC, cob -> { + cob.aload(0); + cob.invokespecial(CD_Object, INIT_NAME, MTD_void); + cob.return_(); + }); + + if (loadable.doesRequireSetup()) { + loadable.setup(clb); + } + + for (var e: extra) { + // always call setup should be no harm + // it suppose to be nop if not required. + e.setup(clb); + } + + clb.withMethodBody("get", MethodTypeDesc.of(CD_Object), ACC_PUBLIC, cob -> { + loadable.load(cob); + cob.areturn(); + }); + }); + } + + void writeClassFile(String className, byte[] classBytes) throws IOException { + if (WRITE_CLASS_FILE) { + Files.write(Path.of(className + ".class"), classBytes); + } + } +} \ No newline at end of file From 94a927f8fd469c57ff2d71f7bbacc5ecc2c1f531 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Sun, 27 Oct 2024 16:50:54 -0700 Subject: [PATCH 07/15] Separate out ModuleDescriptorBuilder and use LoadableArray for paging --- .../jdk/tools/jlink/internal/Snippets.java | 1 - .../internal/plugins/ModuleInfoLoader.java | 514 ++++++++++++++++ .../internal/plugins/SystemModulesPlugin.java | 549 +----------------- 3 files changed, 542 insertions(+), 522 deletions(-) create mode 100644 src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java index c9571a9c11416..167a618cfe66f 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -24,7 +24,6 @@ */ package jdk.tools.jlink.internal; -import java.lang.annotation.Target; import java.lang.classfile.ClassBuilder; import static java.lang.classfile.ClassFile.ACC_PUBLIC; import static java.lang.classfile.ClassFile.ACC_STATIC; diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java new file mode 100644 index 0000000000000..976ca9bbeb339 --- /dev/null +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java @@ -0,0 +1,514 @@ +/* + * Copyright (c) 2024, 2024, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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 jdk.tools.jlink.internal.plugins; + +import java.lang.classfile.ClassBuilder; +import java.lang.classfile.CodeBuilder; +import java.lang.constant.ClassDesc; +import static java.lang.constant.ConstantDescs.*; + +import java.lang.constant.MethodTypeDesc; +import java.lang.module.ModuleDescriptor; +import java.lang.module.ModuleDescriptor.Exports; +import java.lang.module.ModuleDescriptor.Opens; +import java.lang.module.ModuleDescriptor.Provides; +import java.lang.module.ModuleDescriptor.Requires; +import java.lang.module.ModuleDescriptor.Version; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.function.Consumer; + +import jdk.tools.jlink.internal.Snippets.ElementLoader; +import jdk.tools.jlink.internal.Snippets.Loadable; +import jdk.tools.jlink.internal.Snippets.LoadableArray; +import jdk.tools.jlink.internal.Snippets.LoadableSet; +import static jdk.tools.jlink.internal.Snippets.STRING_LOADER; +import static jdk.tools.jlink.internal.Snippets.STRING_PAGE_SIZE; + +import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.ModuleInfo; +import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.SystemModulesClassGenerator.DedupSetBuilder; + +class ModuleInfoLoader implements ElementLoader { + private static final ClassDesc CD_MODULE_DESCRIPTOR = + ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor"); + private static final ClassDesc CD_MODULE_BUILDER = + ClassDesc.ofInternalName("jdk/internal/module/Builder"); + + private static final int PAGING_THRESHOLD = 512; + private final DedupSetBuilder dedupSetBuilder; + private final ClassDesc ownerClassDesc; + + ModuleInfoLoader(DedupSetBuilder dedupSetBuilder, ClassDesc ownerClassDesc) { + this.dedupSetBuilder = dedupSetBuilder; + this.ownerClassDesc = ownerClassDesc; + } + + @Override + public void load(CodeBuilder cob, ModuleInfo moduleInfo, int index) { + var mdBuilder = new ModuleDescriptorBuilder(cob, moduleInfo.descriptor(), moduleInfo.packages(), index); + mdBuilder.load(); + } + + class ModuleDescriptorBuilder { + static final ClassDesc CD_EXPORTS = + ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Exports"); + static final ClassDesc CD_OPENS = + ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Opens"); + static final ClassDesc CD_PROVIDES = + ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Provides"); + static final ClassDesc CD_REQUIRES = + ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Requires"); + + // method signature for static Builder::newExports, newOpens, + // newProvides, newRequires methods + static final MethodTypeDesc MTD_EXPORTS_MODIFIER_SET_STRING_SET = + MethodTypeDesc.of(CD_EXPORTS, CD_Set, CD_String, CD_Set); + static final MethodTypeDesc MTD_EXPORTS_MODIFIER_SET_STRING = + MethodTypeDesc.of(CD_EXPORTS, CD_Set, CD_String); + static final MethodTypeDesc MTD_OPENS_MODIFIER_SET_STRING_SET = + MethodTypeDesc.of(CD_OPENS, CD_Set, CD_String, CD_Set); + static final MethodTypeDesc MTD_OPENS_MODIFIER_SET_STRING = + MethodTypeDesc.of(CD_OPENS, CD_Set, CD_String); + static final MethodTypeDesc MTD_PROVIDES_STRING_LIST = + MethodTypeDesc.of(CD_PROVIDES, CD_String, CD_List); + static final MethodTypeDesc MTD_REQUIRES_SET_STRING = + MethodTypeDesc.of(CD_REQUIRES, CD_Set, CD_String); + static final MethodTypeDesc MTD_REQUIRES_SET_STRING_STRING = + MethodTypeDesc.of(CD_REQUIRES, CD_Set, CD_String, CD_String); + + // method signature for Builder instance methods that + // return this Builder instance + static final MethodTypeDesc MTD_EXPORTS_ARRAY = + MethodTypeDesc.of(CD_MODULE_BUILDER, CD_EXPORTS.arrayType()); + static final MethodTypeDesc MTD_OPENS_ARRAY = + MethodTypeDesc.of(CD_MODULE_BUILDER, CD_OPENS.arrayType()); + static final MethodTypeDesc MTD_PROVIDES_ARRAY = + MethodTypeDesc.of(CD_MODULE_BUILDER, CD_PROVIDES.arrayType()); + static final MethodTypeDesc MTD_REQUIRES_ARRAY = + MethodTypeDesc.of(CD_MODULE_BUILDER, CD_REQUIRES.arrayType()); + static final MethodTypeDesc MTD_SET = MethodTypeDesc.of(CD_MODULE_BUILDER, CD_Set); + static final MethodTypeDesc MTD_STRING = MethodTypeDesc.of(CD_MODULE_BUILDER, CD_String); + static final MethodTypeDesc MTD_BOOLEAN = MethodTypeDesc.of(CD_MODULE_BUILDER, CD_boolean); + static final MethodTypeDesc MTD_void_String = MethodTypeDesc.of(CD_void, CD_String); + static final MethodTypeDesc MTD_ModuleDescriptor_int = MethodTypeDesc.of(CD_MODULE_DESCRIPTOR, CD_int); + static final MethodTypeDesc MTD_List_ObjectArray = MethodTypeDesc.of(CD_List, CD_Object.arrayType()); + + + final CodeBuilder cob; + final ModuleDescriptor md; + final Set packages; + final int index; + Consumer amendment; + + ModuleDescriptorBuilder(CodeBuilder cob, ModuleDescriptor md, Set packages, int index) { + if (md.isAutomatic()) { + throw new InternalError("linking automatic module is not supported"); + } + this.cob = cob; + this.md = md; + this.packages = packages; + this.index = index; + } + + private void setupLoadable(Loadable loadable) { + if (amendment == null) { + amendment = loadable::setup; + } else { + amendment = amendment.andThen(loadable::setup); + } + } + + void setup(ClassBuilder clb) { + amendment.accept(clb); + } + + void load() { + // new jdk.internal.module.Builder + newBuilder(); + + // requires + requires(md.requires()); + + // exports + exports(md.exports()); + + // opens + opens(md.opens()); + + // uses + uses(md.uses()); + + // provides + provides(md.provides()); + + // all packages + packages(packages); + + // version + md.version().ifPresent(this::version); + + // main class + md.mainClass().ifPresent(this::mainClass); + + loadModuleDescriptor(); + } + + void newBuilder() { + cob.new_(CD_MODULE_BUILDER) + .dup() + .loadConstant(md.name()) + .invokespecial(CD_MODULE_BUILDER, + INIT_NAME, + MTD_void_String); + + if (md.isOpen()) { + setModuleBit("open", true); + } + if (md.modifiers().contains(ModuleDescriptor.Modifier.SYNTHETIC)) { + setModuleBit("synthetic", true); + } + if (md.modifiers().contains(ModuleDescriptor.Modifier.MANDATED)) { + setModuleBit("mandated", true); + } + } + + /* + * Invoke Builder.(boolean value) + */ + void setModuleBit(String methodName, boolean value) { + cob.dup() + .loadConstant(value ? 1 : 0) + .invokevirtual(CD_MODULE_BUILDER, + methodName, + MTD_BOOLEAN) + .pop(); + } + + /* + * Put ModuleDescriptor into the modules array + */ + void loadModuleDescriptor() { + cob + .loadConstant(md.hashCode()) + .invokevirtual(CD_MODULE_BUILDER, + "build", + MTD_ModuleDescriptor_int); + } + + /* + * Call Builder::newRequires to create Requires instances and + * then pass it to the builder by calling: + * Builder.requires(Requires[]) + * + */ + void requires(Set requires) { + cob.dup() + .loadConstant(requires.size()) + .anewarray(CD_REQUIRES); + int arrayIndex = 0; + for (Requires require : sorted(requires)) { + String compiledVersion = null; + if (require.compiledVersion().isPresent()) { + compiledVersion = require.compiledVersion().get().toString(); + } + + cob.dup() // arrayref + .loadConstant(arrayIndex++); + newRequires(require.modifiers(), require.name(), compiledVersion); + cob.aastore(); + } + cob.invokevirtual(CD_MODULE_BUILDER, + "requires", + MTD_REQUIRES_ARRAY) + .pop(); + } + + /* + * Invoke Builder.newRequires(Set mods, String mn, String compiledVersion) + * + * Set mods = ... + * Builder.newRequires(mods, mn, compiledVersion); + */ + void newRequires(Set mods, String name, String compiledVersion) { + dedupSetBuilder.loadRequiresModifiers(cob, mods); + cob.loadConstant(name); + if (compiledVersion != null) { + cob.loadConstant(compiledVersion) + .invokestatic(CD_MODULE_BUILDER, + "newRequires", + MTD_REQUIRES_SET_STRING_STRING); + } else { + cob.invokestatic(CD_MODULE_BUILDER, + "newRequires", + MTD_REQUIRES_SET_STRING); + } + } + + /* + * Call Builder::newExports to create Exports instances and + * then pass it to the builder by calling: + * Builder.exports(Exports[]) + * + */ + void exports(Set exports) { + var exportArray = LoadableArray.of( + CD_EXPORTS, + sorted(exports), + this::loadExports, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Exports", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(exportArray); + + cob.dup(); + exportArray.load(cob); + cob.invokevirtual(CD_MODULE_BUILDER, + "exports", + MTD_EXPORTS_ARRAY) + .pop(); + } + + /* + * Invoke + * Builder.newExports(Set ms, String pn, + * Set targets) + * or + * Builder.newExports(Set ms, String pn) + * + * ms = export.modifiers() + * pn = export.source() + * targets = export.targets() + */ + void loadExports(CodeBuilder cb, Exports export, int unused) { + dedupSetBuilder.loadExportsModifiers(cb, export.modifiers()); + cb.loadConstant(export.source()); + var targets = export.targets(); + if (!targets.isEmpty()) { + dedupSetBuilder.loadStringSet(cb, targets); + cb.invokestatic(CD_MODULE_BUILDER, + "newExports", + MTD_EXPORTS_MODIFIER_SET_STRING_SET); + } else { + cb.invokestatic(CD_MODULE_BUILDER, + "newExports", + MTD_EXPORTS_MODIFIER_SET_STRING); + } + } + + + /** + * Call Builder::newOpens to create Opens instances and + * then pass it to the builder by calling: + * Builder.opens(Opens[]) + */ + void opens(Set opens) { + var opensArray = LoadableArray.of( + CD_OPENS, + sorted(opens), + this::newOpens, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Opens", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(opensArray); + + cob.dup(); + opensArray.load(cob); + cob.invokevirtual(CD_MODULE_BUILDER, + "opens", + MTD_OPENS_ARRAY) + .pop(); + } + + /* + * Invoke + * Builder.newOpens(Set ms, String pn, + * Set targets) + * or + * Builder.newOpens(Set ms, String pn) + * + * ms = open.modifiers() + * pn = open.source() + * targets = open.targets() + * Builder.newOpens(mods, pn, targets); + */ + void newOpens(CodeBuilder cb, Opens open, int unused) { + dedupSetBuilder.loadOpensModifiers(cb, open.modifiers()); + cb.loadConstant(open.source()); + var targets = open.targets(); + if (!targets.isEmpty()) { + dedupSetBuilder.loadStringSet(cb, targets); + cb.invokestatic(CD_MODULE_BUILDER, + "newOpens", + MTD_OPENS_MODIFIER_SET_STRING_SET); + } else { + cb.invokestatic(CD_MODULE_BUILDER, + "newOpens", + MTD_OPENS_MODIFIER_SET_STRING); + } + } + + /* + * Invoke Builder.uses(Set uses) + */ + void uses(Set uses) { + cob.dup(); + dedupSetBuilder.loadStringSet(cob, uses); + cob.invokevirtual(CD_MODULE_BUILDER, + "uses", + MTD_SET) + .pop(); + } + + /* + * Call Builder::newProvides to create Provides instances and + * then pass it to the builder by calling: + * Builder.provides(Provides[] provides) + * + */ + void provides(Collection provides) { + var providesArray = LoadableArray.of( + CD_PROVIDES, + sorted(provides), + this::newProvides, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Provides", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(providesArray); + + cob.dup(); + providesArray.load(cob); + cob.invokevirtual(CD_MODULE_BUILDER, + "provides", + MTD_PROVIDES_ARRAY) + .pop(); + } + + /* + * Invoke Builder.newProvides(String service, List providers) + * + * service = provide.service() + * providers = List.of(new String[] { provide.providers() } + * Builder.newProvides(service, providers); + */ + void newProvides(CodeBuilder cb, Provides provide, int offset) { + var providersArray = LoadableArray.of( + CD_String, + provide.providers(), + STRING_LOADER, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Provider" + offset, + STRING_PAGE_SIZE); + + + setupLoadable(providersArray); + + cb.loadConstant(provide.service()); + providersArray.load(cb); + cb.invokestatic(CD_List, + "of", + MTD_List_ObjectArray, + true) + .invokestatic(CD_MODULE_BUILDER, + "newProvides", + MTD_PROVIDES_STRING_LIST); + } + + /* + * Invoke Builder.packages(Set packages) + * with packages either from invoke provider method + * modulePackages() + * or construct inline with + * Set.of(packages) + */ + void packages(Set packages) { + var packagesArray = LoadableSet.of( + sorted(packages), + STRING_LOADER, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Packages", + STRING_PAGE_SIZE); + + setupLoadable(packagesArray); + + cob.dup(); + packagesArray.load(cob); + cob.invokevirtual(CD_MODULE_BUILDER, + "packages", + MTD_SET) + .pop(); + } + + /* + * Invoke Builder.mainClass(String cn) + */ + void mainClass(String cn) { + cob.dup() + .loadConstant(cn) + .invokevirtual(CD_MODULE_BUILDER, + "mainClass", + MTD_STRING) + .pop(); + } + + /* + * Invoke Builder.version(Version v); + */ + void version(Version v) { + cob.dup() + .loadConstant(v.toString()) + .invokevirtual(CD_MODULE_BUILDER, + "version", + MTD_STRING) + .pop(); + } + } + + /** + * Returns a sorted copy of a collection. + * + * This is useful to ensure a deterministic iteration order. + * + * @return a sorted copy of the given collection. + */ + private static > List sorted(Collection c) { + var l = new ArrayList(c); + Collections.sort(l); + return l; + } +} \ No newline at end of file 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 b09622269072d..0838f34bff264 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 @@ -37,7 +37,6 @@ import java.lang.module.ModuleDescriptor.Opens; import java.lang.module.ModuleDescriptor.Provides; import java.lang.module.ModuleDescriptor.Requires; -import java.lang.module.ModuleDescriptor.Version; import java.lang.module.ModuleFinder; import java.lang.module.ModuleReader; import java.lang.module.ModuleReference; @@ -58,7 +57,6 @@ import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; -import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -79,6 +77,9 @@ import java.lang.classfile.TypeKind; import java.lang.classfile.CodeBuilder; +import java.nio.file.Files; +import java.nio.file.Path; + import jdk.tools.jlink.internal.ModuleSorter; import jdk.tools.jlink.plugin.PluginException; import jdk.tools.jlink.plugin.ResourcePool; @@ -332,6 +333,17 @@ private String genSystemModulesClass(List moduleInfos, SystemModulesClassGenerator generator = new SystemModulesClassGenerator(className, moduleInfos, moduleDescriptorsPerMethod); byte[] bytes = generator.genClassBytes(cf); + // Diagnosis help, can be removed + if (Boolean.parseBoolean(System.getProperty("JlinkDumpSystemModuleClass", "false"))) { + try { + var filePath = Path.of(className + ".class").toAbsolutePath(); + System.err.println("Write " + filePath.toString()); + Files.createDirectories(filePath.getParent()); + Files.write(filePath, bytes); + } catch (IOException ioe) { + ioe.printStackTrace(System.err); + } + } String rn = "/java.base/" + className + ".class"; ResourcePoolEntry e = ResourcePoolEntry.create(rn, bytes); out.add(e); @@ -516,8 +528,6 @@ byte[] getBytes() throws IOException { static class SystemModulesClassGenerator { private static final ClassDesc CD_MODULE_DESCRIPTOR = ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor"); - private static final ClassDesc CD_MODULE_BUILDER = - ClassDesc.ofInternalName("jdk/internal/module/Builder"); private static final ClassDesc CD_REQUIRES_MODIFIER = ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Requires$Modifier"); private static final ClassDesc CD_EXPORTS_MODIFIER = @@ -542,10 +552,10 @@ static class SystemModulesClassGenerator { private static final MethodTypeDesc MTD_MapEntry_Object_Object = MethodTypeDesc.of(CD_Map_Entry, CD_Object, CD_Object); private static final MethodTypeDesc MTD_Map_MapEntryArray = MethodTypeDesc.of(CD_Map, CD_Map_Entry.arrayType()); + static final int PAGING_THRESHOLD = 512; // An arbitrary number as this likely generate minimum ~4K code private static final int MAX_LOCAL_VARS = 256; - 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 final int BUILDER_VAR = 2; @@ -558,8 +568,6 @@ static class SystemModulesClassGenerator { private final int moduleDescriptorsPerMethod; - private final ArrayList> amendments = new ArrayList<>(); - // 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 @@ -638,18 +646,9 @@ public byte[] genClassBytes(Configuration cf) { // generate moduleReads genModuleReads(clb, cf); - - // generate module helpers - amendments.forEach(amendment -> amendment.accept(clb)); }); } - private void setupLoadable(Loadable loadable) { - if (loadable.doesRequireSetup()) { - amendments.add(loadable::setup); - } - } - /** * Generate bytecode for no-arg constructor */ @@ -715,101 +714,26 @@ private void genIncubatorModules(ClassBuilder clb) { .ireturn()); } - /** - * Generate bytecode for moduleDescriptors method - */ private void genModuleDescriptorsMethod(ClassBuilder clb) { - if (moduleInfos.size() <= moduleDescriptorsPerMethod) { - clb.withMethodBody( - "moduleDescriptors", - MTD_ModuleDescriptorArray, - ACC_PUBLIC, - cob -> { - cob.loadConstant(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; - } + var moduleDescriptors = LoadableArray.of( + CD_MODULE_DESCRIPTOR, + moduleInfos, + new ModuleInfoLoader(dedupSetBuilder, classDesc), + moduleDescriptorsPerMethod, + classDesc, + "sub", + moduleDescriptorsPerMethod); - - // 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. - 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"; + moduleDescriptors.setup(clb); clb.withMethodBody( "moduleDescriptors", MTD_ModuleDescriptorArray, ACC_PUBLIC, cob -> { - cob.loadConstant(moduleInfos.size()) - .anewarray(CD_MODULE_DESCRIPTOR) - .dup() - .astore(MD_VAR); - cob.aload(0) - .aload(MD_VAR) - .invokevirtual( - this.classDesc, - helperMethodNamePrefix + "0", - MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType()) - ) - .areturn(); + moduleDescriptors.load(cob); + cob.areturn(); }); - - 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 - clb.withMethodBody( - helperMethodNamePrefix + index, - MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType()), - ACC_PUBLIC, - cob -> { - 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) { - cob.aload(0) - .aload(MD_VAR) - .invokevirtual( - this.classDesc, - helperMethodNamePrefix + (index+1), - MethodTypeDesc.of(CD_void, CD_MODULE_DESCRIPTOR.arrayType()) - ); - } - - cob.return_(); - }); - } } /** @@ -1021,423 +945,6 @@ private void genImmutableSet(CodeBuilder cob, Set set) { loadableSet.load(cob); } - class ModuleDescriptorBuilder { - static final ClassDesc CD_EXPORTS = - ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Exports"); - static final ClassDesc CD_OPENS = - ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Opens"); - static final ClassDesc CD_PROVIDES = - ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Provides"); - static final ClassDesc CD_REQUIRES = - ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Requires"); - - // method signature for static Builder::newExports, newOpens, - // newProvides, newRequires methods - static final MethodTypeDesc MTD_EXPORTS_MODIFIER_SET_STRING_SET = - MethodTypeDesc.of(CD_EXPORTS, CD_Set, CD_String, CD_Set); - static final MethodTypeDesc MTD_EXPORTS_MODIFIER_SET_STRING = - MethodTypeDesc.of(CD_EXPORTS, CD_Set, CD_String); - static final MethodTypeDesc MTD_OPENS_MODIFIER_SET_STRING_SET = - MethodTypeDesc.of(CD_OPENS, CD_Set, CD_String, CD_Set); - static final MethodTypeDesc MTD_OPENS_MODIFIER_SET_STRING = - MethodTypeDesc.of(CD_OPENS, CD_Set, CD_String); - static final MethodTypeDesc MTD_PROVIDES_STRING_LIST = - MethodTypeDesc.of(CD_PROVIDES, CD_String, CD_List); - static final MethodTypeDesc MTD_REQUIRES_SET_STRING = - MethodTypeDesc.of(CD_REQUIRES, CD_Set, CD_String); - static final MethodTypeDesc MTD_REQUIRES_SET_STRING_STRING = - MethodTypeDesc.of(CD_REQUIRES, CD_Set, CD_String, CD_String); - - // method signature for Builder instance methods that - // return this Builder instance - static final MethodTypeDesc MTD_EXPORTS_ARRAY = - MethodTypeDesc.of(CD_MODULE_BUILDER, CD_EXPORTS.arrayType()); - static final MethodTypeDesc MTD_OPENS_ARRAY = - MethodTypeDesc.of(CD_MODULE_BUILDER, CD_OPENS.arrayType()); - static final MethodTypeDesc MTD_PROVIDES_ARRAY = - MethodTypeDesc.of(CD_MODULE_BUILDER, CD_PROVIDES.arrayType()); - static final MethodTypeDesc MTD_REQUIRES_ARRAY = - MethodTypeDesc.of(CD_MODULE_BUILDER, CD_REQUIRES.arrayType()); - static final MethodTypeDesc MTD_SET = MethodTypeDesc.of(CD_MODULE_BUILDER, CD_Set); - static final MethodTypeDesc MTD_STRING = MethodTypeDesc.of(CD_MODULE_BUILDER, CD_String); - static final MethodTypeDesc MTD_BOOLEAN = MethodTypeDesc.of(CD_MODULE_BUILDER, CD_boolean); - static final MethodTypeDesc MTD_void_String = MethodTypeDesc.of(CD_void, CD_String); - static final MethodTypeDesc MTD_ModuleDescriptor_int = MethodTypeDesc.of(CD_MODULE_DESCRIPTOR, CD_int); - static final MethodTypeDesc MTD_List_ObjectArray = MethodTypeDesc.of(CD_List, CD_Object.arrayType()); - - static final int PAGING_THRESHOLD = 512; // An arbitrary number as this likely generate minimum ~4K code - - final CodeBuilder cob; - final ModuleDescriptor md; - final Set packages; - final int index; - - ModuleDescriptorBuilder(CodeBuilder cob, ModuleDescriptor md, Set packages, int index) { - if (md.isAutomatic()) { - throw new InternalError("linking automatic module is not supported"); - } - this.cob = cob; - this.md = md; - this.packages = packages; - this.index = index; - } - - void build() { - // new jdk.internal.module.Builder - newBuilder(); - - // requires - requires(md.requires()); - - // exports - exports(md.exports()); - - // opens - opens(md.opens()); - - // uses - uses(md.uses()); - - // provides - provides(md.provides()); - - // all packages - packages(packages); - - // version - md.version().ifPresent(this::version); - - // main class - md.mainClass().ifPresent(this::mainClass); - - putModuleDescriptor(); - } - - void newBuilder() { - cob.new_(CD_MODULE_BUILDER) - .dup() - .loadConstant(md.name()) - .invokespecial(CD_MODULE_BUILDER, - INIT_NAME, - MTD_void_String) - .astore(BUILDER_VAR); - - if (md.isOpen()) { - setModuleBit("open", true); - } - if (md.modifiers().contains(ModuleDescriptor.Modifier.SYNTHETIC)) { - setModuleBit("synthetic", true); - } - if (md.modifiers().contains(ModuleDescriptor.Modifier.MANDATED)) { - setModuleBit("mandated", true); - } - } - - /* - * Invoke Builder.(boolean value) - */ - void setModuleBit(String methodName, boolean value) { - cob.aload(BUILDER_VAR) - .loadConstant(value ? 1 : 0) - .invokevirtual(CD_MODULE_BUILDER, - methodName, - MTD_BOOLEAN) - .pop(); - } - - /* - * Put ModuleDescriptor into the modules array - */ - void putModuleDescriptor() { - cob.aload(MD_VAR) - .loadConstant(index) - .aload(BUILDER_VAR) - .loadConstant(md.hashCode()) - .invokevirtual(CD_MODULE_BUILDER, - "build", - MTD_ModuleDescriptor_int) - .aastore(); - } - - /* - * Call Builder::newRequires to create Requires instances and - * then pass it to the builder by calling: - * Builder.requires(Requires[]) - * - */ - void requires(Set requires) { - cob.aload(BUILDER_VAR) - .loadConstant(requires.size()) - .anewarray(CD_REQUIRES); - int arrayIndex = 0; - for (Requires require : sorted(requires)) { - String compiledVersion = null; - if (require.compiledVersion().isPresent()) { - compiledVersion = require.compiledVersion().get().toString(); - } - - cob.dup() // arrayref - .loadConstant(arrayIndex++); - newRequires(require.modifiers(), require.name(), compiledVersion); - cob.aastore(); - } - cob.invokevirtual(CD_MODULE_BUILDER, - "requires", - MTD_REQUIRES_ARRAY) - .pop(); - } - - /* - * Invoke Builder.newRequires(Set mods, String mn, String compiledVersion) - * - * Set mods = ... - * Builder.newRequires(mods, mn, compiledVersion); - */ - void newRequires(Set mods, String name, String compiledVersion) { - dedupSetBuilder.loadRequiresModifiers(cob, mods); - cob.loadConstant(name); - if (compiledVersion != null) { - cob.loadConstant(compiledVersion) - .invokestatic(CD_MODULE_BUILDER, - "newRequires", - MTD_REQUIRES_SET_STRING_STRING); - } else { - cob.invokestatic(CD_MODULE_BUILDER, - "newRequires", - MTD_REQUIRES_SET_STRING); - } - } - - /* - * Call Builder::newExports to create Exports instances and - * then pass it to the builder by calling: - * Builder.exports(Exports[]) - * - */ - void exports(Set exports) { - var exportArray = LoadableArray.of( - CD_EXPORTS, - sorted(exports), - this::loadExports, - PAGING_THRESHOLD, - classDesc, - "module" + index + "Exports", - // number safe for a single page helper under 64K size limit - 2000); - - setupLoadable(exportArray); - - cob.aload(BUILDER_VAR); - exportArray.load(cob); - cob.invokevirtual(CD_MODULE_BUILDER, - "exports", - MTD_EXPORTS_ARRAY) - .pop(); - } - - /* - * Invoke - * Builder.newExports(Set ms, String pn, - * Set targets) - * or - * Builder.newExports(Set ms, String pn) - * - * ms = export.modifiers() - * pn = export.source() - * targets = export.targets() - */ - void loadExports(CodeBuilder cb, Exports export, int unused) { - dedupSetBuilder.loadExportsModifiers(cb, export.modifiers()); - cb.loadConstant(export.source()); - var targets = export.targets(); - if (!targets.isEmpty()) { - dedupSetBuilder.loadStringSet(cb, targets); - cb.invokestatic(CD_MODULE_BUILDER, - "newExports", - MTD_EXPORTS_MODIFIER_SET_STRING_SET); - } else { - cb.invokestatic(CD_MODULE_BUILDER, - "newExports", - MTD_EXPORTS_MODIFIER_SET_STRING); - } - } - - - /** - * Call Builder::newOpens to create Opens instances and - * then pass it to the builder by calling: - * Builder.opens(Opens[]) - */ - void opens(Set opens) { - var opensArray = LoadableArray.of( - CD_OPENS, - sorted(opens), - this::newOpens, - PAGING_THRESHOLD, - classDesc, - "module" + index + "Opens", - // number safe for a single page helper under 64K size limit - 2000); - - setupLoadable(opensArray); - - cob.aload(BUILDER_VAR); - opensArray.load(cob); - cob.invokevirtual(CD_MODULE_BUILDER, - "opens", - MTD_OPENS_ARRAY) - .pop(); - } - - /* - * Invoke - * Builder.newOpens(Set ms, String pn, - * Set targets) - * or - * Builder.newOpens(Set ms, String pn) - * - * ms = open.modifiers() - * pn = open.source() - * targets = open.targets() - * Builder.newOpens(mods, pn, targets); - */ - void newOpens(CodeBuilder cb, Opens open, int unused) { - dedupSetBuilder.loadOpensModifiers(cb, open.modifiers()); - cb.loadConstant(open.source()); - var targets = open.targets(); - if (!targets.isEmpty()) { - dedupSetBuilder.loadStringSet(cb, targets); - cb.invokestatic(CD_MODULE_BUILDER, - "newOpens", - MTD_OPENS_MODIFIER_SET_STRING_SET); - } else { - cb.invokestatic(CD_MODULE_BUILDER, - "newOpens", - MTD_OPENS_MODIFIER_SET_STRING); - } - } - - /* - * Invoke Builder.uses(Set uses) - */ - void uses(Set uses) { - cob.aload(BUILDER_VAR); - dedupSetBuilder.loadStringSet(cob, uses); - cob.invokevirtual(CD_MODULE_BUILDER, - "uses", - MTD_SET) - .pop(); - } - - /* - * Call Builder::newProvides to create Provides instances and - * then pass it to the builder by calling: - * Builder.provides(Provides[] provides) - * - */ - void provides(Collection provides) { - var providesArray = LoadableArray.of( - CD_PROVIDES, - sorted(provides), - this::newProvides, - PAGING_THRESHOLD, - classDesc, - "module" + index + "Provides", - // number safe for a single page helper under 64K size limit - 2000); - - setupLoadable(providesArray); - - cob.aload(BUILDER_VAR); - providesArray.load(cob); - cob.invokevirtual(CD_MODULE_BUILDER, - "provides", - MTD_PROVIDES_ARRAY) - .pop(); - } - - /* - * Invoke Builder.newProvides(String service, List providers) - * - * service = provide.service() - * providers = List.of(new String[] { provide.providers() } - * Builder.newProvides(service, providers); - */ - void newProvides(CodeBuilder cb, Provides provide, int offset) { - var providersArray = LoadableArray.of( - CD_String, - provide.providers(), - STRING_LOADER, - PAGING_THRESHOLD, - classDesc, - "module" + index + "Provider" + offset, - STRING_PAGE_SIZE); - - - setupLoadable(providersArray); - - cb.loadConstant(provide.service()); - providersArray.load(cb); - cb.invokestatic(CD_List, - "of", - MTD_List_ObjectArray, - true) - .invokestatic(CD_MODULE_BUILDER, - "newProvides", - MTD_PROVIDES_STRING_LIST); - } - - /* - * Invoke Builder.packages(Set packages) - * with packages either from invoke provider method - * modulePackages() - * or construct inline with - * Set.of(packages) - */ - void packages(Set packages) { - var packagesArray = LoadableSet.of( - sorted(packages), - STRING_LOADER, - PAGING_THRESHOLD, - classDesc, - "module" + index + "Packages", - STRING_PAGE_SIZE); - - setupLoadable(packagesArray); - - cob.aload(BUILDER_VAR); - packagesArray.load(cob); - cob.invokevirtual(CD_MODULE_BUILDER, - "packages", - MTD_SET) - .pop(); - } - - /* - * Invoke Builder.mainClass(String cn) - */ - void mainClass(String cn) { - cob.aload(BUILDER_VAR) - .loadConstant(cn) - .invokevirtual(CD_MODULE_BUILDER, - "mainClass", - MTD_STRING) - .pop(); - } - - /* - * Invoke Builder.version(Version v); - */ - void version(Version v) { - cob.aload(BUILDER_VAR) - .loadConstant(v.toString()) - .invokevirtual(CD_MODULE_BUILDER, - "version", - MTD_STRING) - .pop(); - } - } - class ModuleHashesBuilder { private static final ClassDesc MODULE_HASHES_BUILDER = ClassDesc.ofInternalName("jdk/internal/module/ModuleHashes$Builder"); @@ -1565,7 +1072,7 @@ static class DedupSetBuilder { > SetReference createLoadableSet(Set elements, ElementLoader elementLoader) { var loadableSet = LoadableSet.of(sorted(elements), elementLoader, - ModuleDescriptorBuilder.PAGING_THRESHOLD, + PAGING_THRESHOLD, owner, "dedupSet" + values.size(), // Safe for String and Enum within 64K @@ -1684,7 +1191,7 @@ Optional> buildConstants(ClassBuilder clb) { CD_Set, staticCache, cacheLoader, - ModuleDescriptorBuilder.PAGING_THRESHOLD, + PAGING_THRESHOLD, owner, VALUES_ARRAY, 2000); From 0ba821ae5605762d5edbaa89413edc75e2331cce Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Sun, 27 Oct 2024 22:12:21 -0700 Subject: [PATCH 08/15] Fix regression failed to setup helper methods properly --- .../internal/plugins/ModuleInfoLoader.java | 377 +++++++----------- .../internal/plugins/SystemModulesPlugin.java | 8 +- 2 files changed, 157 insertions(+), 228 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java index 976ca9bbeb339..d0e8a08caece2 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java @@ -35,11 +35,11 @@ import java.lang.module.ModuleDescriptor.Opens; import java.lang.module.ModuleDescriptor.Provides; import java.lang.module.ModuleDescriptor.Requires; -import java.lang.module.ModuleDescriptor.Version; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.function.Consumer; @@ -62,6 +62,7 @@ class ModuleInfoLoader implements ElementLoader { private static final int PAGING_THRESHOLD = 512; private final DedupSetBuilder dedupSetBuilder; private final ClassDesc ownerClassDesc; + private final ArrayList> amendments = new ArrayList<>(); ModuleInfoLoader(DedupSetBuilder dedupSetBuilder, ClassDesc ownerClassDesc) { this.dedupSetBuilder = dedupSetBuilder; @@ -70,8 +71,15 @@ class ModuleInfoLoader implements ElementLoader { @Override public void load(CodeBuilder cob, ModuleInfo moduleInfo, int index) { - var mdBuilder = new ModuleDescriptorBuilder(cob, moduleInfo.descriptor(), moduleInfo.packages(), index); - mdBuilder.load(); + var mdBuilder = new ModuleDescriptorBuilder(moduleInfo.descriptor(), moduleInfo.packages(), index); + mdBuilder.load(cob); + if (mdBuilder.doesRequireSetup()) { + amendments.add(mdBuilder::setup); + } + } + + public void finish(ClassBuilder clb) { + amendments.forEach(a -> a.accept(clb)); } class ModuleDescriptorBuilder { @@ -119,22 +127,95 @@ class ModuleDescriptorBuilder { static final MethodTypeDesc MTD_List_ObjectArray = MethodTypeDesc.of(CD_List, CD_Object.arrayType()); - final CodeBuilder cob; final ModuleDescriptor md; final Set packages; final int index; Consumer amendment; - ModuleDescriptorBuilder(CodeBuilder cob, ModuleDescriptor md, Set packages, int index) { + ModuleDescriptorBuilder(ModuleDescriptor md, Set packages, int index) { if (md.isAutomatic()) { throw new InternalError("linking automatic module is not supported"); } - this.cob = cob; + + this.md = md; this.packages = packages; this.index = index; } + private LoadableArray requiresArray() { + var requiresArray = LoadableArray.of( + CD_REQUIRES, + sorted(md.requires()), + this::newRequires, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Requires", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(requiresArray); + return requiresArray; + } + + private LoadableArray exportArray() { + var exportArray = LoadableArray.of( + CD_EXPORTS, + sorted(md.exports()), + this::loadExports, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Exports", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(exportArray); + return exportArray; + } + + private LoadableArray opensArray() { + var opensArray = LoadableArray.of( + CD_OPENS, + sorted(md.opens()), + this::newOpens, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Opens", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(opensArray); + return opensArray; + } + + private LoadableArray providesArray() { + var providesArray = LoadableArray.of( + CD_PROVIDES, + sorted(md.provides()), + this::newProvides, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Provides", + // number safe for a single page helper under 64K size limit + 2000); + + setupLoadable(providesArray); + return providesArray; + } + + private LoadableSet packagesSet() { + var packagesSet = LoadableSet.of( + sorted(packages), + STRING_LOADER, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Packages", + STRING_PAGE_SIZE); + + setupLoadable(packagesSet); + return packagesSet; + } + private void setupLoadable(Loadable loadable) { if (amendment == null) { amendment = loadable::setup; @@ -143,109 +224,95 @@ private void setupLoadable(Loadable loadable) { } } + boolean doesRequireSetup() { + return amendment != null; + } + void setup(ClassBuilder clb) { - amendment.accept(clb); + if (amendment != null) amendment.accept(clb); } - void load() { + void load(CodeBuilder cob) { // new jdk.internal.module.Builder - newBuilder(); + cob.new_(CD_MODULE_BUILDER) + .dup() + .loadConstant(md.name()) + .invokespecial(CD_MODULE_BUILDER, + INIT_NAME, + MTD_void_String); + if (md.isOpen()) { + setModuleBit(cob, "open", true); + } + if (md.modifiers().contains(ModuleDescriptor.Modifier.SYNTHETIC)) { + setModuleBit(cob, "synthetic", true); + } + if (md.modifiers().contains(ModuleDescriptor.Modifier.MANDATED)) { + setModuleBit(cob, "mandated", true); + } // requires - requires(md.requires()); + requiresArray().load(cob); + cob.invokevirtual(CD_MODULE_BUILDER, + "requires", + MTD_REQUIRES_ARRAY); // exports - exports(md.exports()); + exportArray().load(cob); + cob.invokevirtual(CD_MODULE_BUILDER, + "exports", + MTD_EXPORTS_ARRAY); // opens - opens(md.opens()); + opensArray().load(cob); + cob.invokevirtual(CD_MODULE_BUILDER, + "opens", + MTD_OPENS_ARRAY); // uses - uses(md.uses()); + dedupSetBuilder.loadStringSet(cob, md.uses()); + cob.invokevirtual(CD_MODULE_BUILDER, + "uses", + MTD_SET); // provides - provides(md.provides()); + providesArray().load(cob); + cob.invokevirtual(CD_MODULE_BUILDER, + "provides", + MTD_PROVIDES_ARRAY); // all packages - packages(packages); + packagesSet().load(cob); + cob.invokevirtual(CD_MODULE_BUILDER, + "packages", + MTD_SET); // version - md.version().ifPresent(this::version); + md.version().ifPresent(v -> setModuleProperty(cob, "version", v.toString())); // main class - md.mainClass().ifPresent(this::mainClass); - - loadModuleDescriptor(); - } - - void newBuilder() { - cob.new_(CD_MODULE_BUILDER) - .dup() - .loadConstant(md.name()) - .invokespecial(CD_MODULE_BUILDER, - INIT_NAME, - MTD_void_String); + md.mainClass().ifPresent(cn -> setModuleProperty(cob, "mainClass", cn)); - if (md.isOpen()) { - setModuleBit("open", true); - } - if (md.modifiers().contains(ModuleDescriptor.Modifier.SYNTHETIC)) { - setModuleBit("synthetic", true); - } - if (md.modifiers().contains(ModuleDescriptor.Modifier.MANDATED)) { - setModuleBit("mandated", true); - } + cob.loadConstant(md.hashCode()) + .invokevirtual(CD_MODULE_BUILDER, + "build", + MTD_ModuleDescriptor_int); } /* * Invoke Builder.(boolean value) */ - void setModuleBit(String methodName, boolean value) { - cob.dup() - .loadConstant(value ? 1 : 0) + void setModuleBit(CodeBuilder cob, String methodName, boolean value) { + cob.loadConstant(value ? 1 : 0) .invokevirtual(CD_MODULE_BUILDER, methodName, - MTD_BOOLEAN) - .pop(); + MTD_BOOLEAN); } - /* - * Put ModuleDescriptor into the modules array - */ - void loadModuleDescriptor() { - cob - .loadConstant(md.hashCode()) + void setModuleProperty(CodeBuilder cob, String methodName, String value) { + cob.loadConstant(value) .invokevirtual(CD_MODULE_BUILDER, - "build", - MTD_ModuleDescriptor_int); - } - - /* - * Call Builder::newRequires to create Requires instances and - * then pass it to the builder by calling: - * Builder.requires(Requires[]) - * - */ - void requires(Set requires) { - cob.dup() - .loadConstant(requires.size()) - .anewarray(CD_REQUIRES); - int arrayIndex = 0; - for (Requires require : sorted(requires)) { - String compiledVersion = null; - if (require.compiledVersion().isPresent()) { - compiledVersion = require.compiledVersion().get().toString(); - } - - cob.dup() // arrayref - .loadConstant(arrayIndex++); - newRequires(require.modifiers(), require.name(), compiledVersion); - cob.aastore(); - } - cob.invokevirtual(CD_MODULE_BUILDER, - "requires", - MTD_REQUIRES_ARRAY) - .pop(); + methodName, + MTD_STRING); } /* @@ -254,11 +321,11 @@ void requires(Set requires) { * Set mods = ... * Builder.newRequires(mods, mn, compiledVersion); */ - void newRequires(Set mods, String name, String compiledVersion) { - dedupSetBuilder.loadRequiresModifiers(cob, mods); - cob.loadConstant(name); - if (compiledVersion != null) { - cob.loadConstant(compiledVersion) + void newRequires(CodeBuilder cob, Requires require, int unused) { + dedupSetBuilder.loadRequiresModifiers(cob, require.modifiers()); + cob.loadConstant(require.name()); + if (require.compiledVersion().isPresent()) { + cob.loadConstant(require.compiledVersion().get().toString()) .invokestatic(CD_MODULE_BUILDER, "newRequires", MTD_REQUIRES_SET_STRING_STRING); @@ -269,33 +336,6 @@ void newRequires(Set mods, String name, String compiledVersio } } - /* - * Call Builder::newExports to create Exports instances and - * then pass it to the builder by calling: - * Builder.exports(Exports[]) - * - */ - void exports(Set exports) { - var exportArray = LoadableArray.of( - CD_EXPORTS, - sorted(exports), - this::loadExports, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Exports", - // number safe for a single page helper under 64K size limit - 2000); - - setupLoadable(exportArray); - - cob.dup(); - exportArray.load(cob); - cob.invokevirtual(CD_MODULE_BUILDER, - "exports", - MTD_EXPORTS_ARRAY) - .pop(); - } - /* * Invoke * Builder.newExports(Set ms, String pn, @@ -323,33 +363,6 @@ void loadExports(CodeBuilder cb, Exports export, int unused) { } } - - /** - * Call Builder::newOpens to create Opens instances and - * then pass it to the builder by calling: - * Builder.opens(Opens[]) - */ - void opens(Set opens) { - var opensArray = LoadableArray.of( - CD_OPENS, - sorted(opens), - this::newOpens, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Opens", - // number safe for a single page helper under 64K size limit - 2000); - - setupLoadable(opensArray); - - cob.dup(); - opensArray.load(cob); - cob.invokevirtual(CD_MODULE_BUILDER, - "opens", - MTD_OPENS_ARRAY) - .pop(); - } - /* * Invoke * Builder.newOpens(Set ms, String pn, @@ -378,45 +391,6 @@ void newOpens(CodeBuilder cb, Opens open, int unused) { } } - /* - * Invoke Builder.uses(Set uses) - */ - void uses(Set uses) { - cob.dup(); - dedupSetBuilder.loadStringSet(cob, uses); - cob.invokevirtual(CD_MODULE_BUILDER, - "uses", - MTD_SET) - .pop(); - } - - /* - * Call Builder::newProvides to create Provides instances and - * then pass it to the builder by calling: - * Builder.provides(Provides[] provides) - * - */ - void provides(Collection provides) { - var providesArray = LoadableArray.of( - CD_PROVIDES, - sorted(provides), - this::newProvides, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Provides", - // number safe for a single page helper under 64K size limit - 2000); - - setupLoadable(providesArray); - - cob.dup(); - providesArray.load(cob); - cob.invokevirtual(CD_MODULE_BUILDER, - "provides", - MTD_PROVIDES_ARRAY) - .pop(); - } - /* * Invoke Builder.newProvides(String service, List providers) * @@ -434,7 +408,6 @@ void newProvides(CodeBuilder cb, Provides provide, int offset) { "module" + index + "Provider" + offset, STRING_PAGE_SIZE); - setupLoadable(providersArray); cb.loadConstant(provide.service()); @@ -447,56 +420,6 @@ void newProvides(CodeBuilder cb, Provides provide, int offset) { "newProvides", MTD_PROVIDES_STRING_LIST); } - - /* - * Invoke Builder.packages(Set packages) - * with packages either from invoke provider method - * modulePackages() - * or construct inline with - * Set.of(packages) - */ - void packages(Set packages) { - var packagesArray = LoadableSet.of( - sorted(packages), - STRING_LOADER, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Packages", - STRING_PAGE_SIZE); - - setupLoadable(packagesArray); - - cob.dup(); - packagesArray.load(cob); - cob.invokevirtual(CD_MODULE_BUILDER, - "packages", - MTD_SET) - .pop(); - } - - /* - * Invoke Builder.mainClass(String cn) - */ - void mainClass(String cn) { - cob.dup() - .loadConstant(cn) - .invokevirtual(CD_MODULE_BUILDER, - "mainClass", - MTD_STRING) - .pop(); - } - - /* - * Invoke Builder.version(Version v); - */ - void version(Version v) { - cob.dup() - .loadConstant(v.toString()) - .invokevirtual(CD_MODULE_BUILDER, - "version", - MTD_STRING) - .pop(); - } } /** 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 0838f34bff264..64f25f1f62618 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 @@ -715,15 +715,18 @@ private void genIncubatorModules(ClassBuilder clb) { } private void genModuleDescriptorsMethod(ClassBuilder clb) { + var moduleInfoLoader = new ModuleInfoLoader(dedupSetBuilder, classDesc); var moduleDescriptors = LoadableArray.of( CD_MODULE_DESCRIPTOR, moduleInfos, - new ModuleInfoLoader(dedupSetBuilder, classDesc), + moduleInfoLoader, moduleDescriptorsPerMethod, classDesc, "sub", moduleDescriptorsPerMethod); + // This setup helpers needed by the LoadableArray, but element loader is responsible + // to setup elements. moduleDescriptors.setup(clb); clb.withMethodBody( @@ -734,6 +737,9 @@ private void genModuleDescriptorsMethod(ClassBuilder clb) { moduleDescriptors.load(cob); cob.areturn(); }); + + // amend class with helpers needed by individual ModuleDescriptor + moduleInfoLoader.finish(clb); } /** From 910769951fdc9a3c30c54000f08bb621b94a1424 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Tue, 29 Oct 2024 15:38:44 -0700 Subject: [PATCH 09/15] Fix typo and comment to match latest implementation --- .../jdk/tools/jlink/internal/Snippets.java | 2 +- .../internal/plugins/SystemModulesPlugin.java | 22 +++++-------------- test/jdk/tools/jlink/JLink20000Packages.java | 6 ++--- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java index 167a618cfe66f..35eb416d0e608 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -66,7 +66,7 @@ public sealed interface Loadable { void load(CodeBuilder cob); /** - * The type of the reference be loaded onto the operatnd stack. + * The type of the reference be loaded onto the operand stack. */ ClassDesc classDesc(); 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 64f25f1f62618..41215b6a53c93 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 @@ -1211,18 +1211,11 @@ Optional> buildConstants(ClassBuilder clb) { } /* - * SetReference count references to the set, and use an element loader, which is - * a CodeBuilder that generate bytecode snippet to load an element onto the operand - * stack, to generate bytecode to support loading the set onto operand stack. - * - * When a set size is over SET_SIZE_THRESHOLD, a provider function is generated - * to build the set rather than inline to avoid method size overflow. - * - * When a set is referenced more than once, the set value is to be built once - * and cached in an array to be load later. - * - * generateConstant method should be called to setup the provider methods and cache array. - * load method can then be called to load the set onto the operand stack. + * SetReference count references to the set, and use LoadableSet under the hood to + * support paginiation as needed. + * For sets referenced more than once, a cache is used to store the pre-built result + * and load from there. Otherwise, the set is built in place and load onto the operand + * stack. */ class SetReference { // sorted elements of the set to ensure same generated code @@ -1256,10 +1249,7 @@ void assignTo(int index) { // When referenced more than once, the value is pre-built with static initialzer // and is load from the cache array with // dedupSetValues[index] - // Otherwise, built the set in place with either - // Set.of(elements) - // or invoke the generated provider method - // methodName() + // Otherwise, LoadableSet will load the set onto the operand stack. void load(CodeBuilder cob) { if (refCount > 1) { assert index >= 0; diff --git a/test/jdk/tools/jlink/JLink20000Packages.java b/test/jdk/tools/jlink/JLink20000Packages.java index bdb7a42540606..ee24283e00858 100644 --- a/test/jdk/tools/jlink/JLink20000Packages.java +++ b/test/jdk/tools/jlink/JLink20000Packages.java @@ -32,9 +32,9 @@ /* * @test - * @summary Make sure that ~20000 packages in a uber jar can be linked using jlink. Depends on the - * packages, this is almost hit the 64K limitation as each plain export could take - * ~17 bytecodes. + * @summary Make sure that ~20000 packages in a uber jar can be linked using jlink. Now that + * pagination is in place, the limitation is on the constant pool size, not number + * of packages. * @bug 8321413 * @library ../lib * @enablePreview From 69d84664fa4feeb27fdaa5d1d6ba407724becf9f Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Thu, 14 Nov 2024 22:31:51 -0800 Subject: [PATCH 10/15] Rename based on feedback to emphasis building a snippet for loading a reference --- .../jdk/tools/jlink/internal/Snippets.java | 283 +++++++-------- ...ader.java => ModuleDescriptorBuilder.java} | 331 ++++++++---------- .../internal/plugins/SystemModulesPlugin.java | 85 +++-- test/jdk/tools/jlink/SnippetsTest.java | 32 +- 4 files changed, 339 insertions(+), 392 deletions(-) rename src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/{ModuleInfoLoader.java => ModuleDescriptorBuilder.java} (67%) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java index 35eb416d0e608..91acea0dde2ff 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -25,61 +25,91 @@ package jdk.tools.jlink.internal; import java.lang.classfile.ClassBuilder; -import static java.lang.classfile.ClassFile.ACC_PUBLIC; -import static java.lang.classfile.ClassFile.ACC_STATIC; import java.lang.classfile.CodeBuilder; import java.lang.constant.ClassDesc; -import static java.lang.constant.ConstantDescs.CD_Integer; -import static java.lang.constant.ConstantDescs.CD_Object; -import static java.lang.constant.ConstantDescs.CD_Set; -import static java.lang.constant.ConstantDescs.CD_int; +import java.lang.constant.ConstantDesc; import java.lang.constant.MethodTypeDesc; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.function.BiConsumer; - -import jdk.tools.jlink.internal.Snippets.ElementLoader; +import static java.lang.classfile.ClassFile.ACC_PUBLIC; +import static java.lang.classfile.ClassFile.ACC_STATIC; +import static java.lang.constant.ConstantDescs.CD_Integer; +import static java.lang.constant.ConstantDescs.CD_Object; +import static java.lang.constant.ConstantDescs.CD_Set; +import static java.lang.constant.ConstantDescs.CD_int; public class Snippets { // Tested page size of string array public static final int STRING_PAGE_SIZE = 8000; - public static final ElementLoader STRING_LOADER = ElementLoader.of(CodeBuilder::loadConstant); - public static final ElementLoader INTEGER_LOADER = (cob, value, index) -> { + public static final CollectionElementBuilder STRING_LOADER = (value, index) -> new Constant<>(value); + + public static final CollectionElementBuilder INTEGER_LOADER = (value, index) -> cob -> { // loadConstant will unbox cob.loadConstant(value) - .invokestatic(ClassDesc.ofInternalName("java/lang/Integer"), "valueOf", MethodTypeDesc.of(CD_Integer, CD_int)); + .invokestatic(ClassDesc.ofInternalName("java/lang/Integer"), "valueOf", MethodTypeDesc.of(CD_Integer, CD_int)); }; - public static final ElementLoader LOADABLE_LOADER = (cob, loadable, index) -> loadable.load(cob); + + public static final CollectionElementBuilder LOADABLE_LOADER = (loadable, index) -> loadable; + + /** + * Snippet of bytecodes + */ + @FunctionalInterface + public interface Snippet { + /** + * Emit the bytecode snippet to the CodeBuilder. + * + * @param cob The CodeBuilder the bytecode snippet. + * @throws IllegalStateException If the snippet is not setup properly. + */ + void emit(CodeBuilder cob); + + /** + * Perpare a snippet needs some extra support like field or methods from the class. + * + * @param clb The ClassBuilder to setup the helpers. + */ + default void setup(ClassBuilder clb) {}; + } /** * Describe a reference that can be load onto the operand stack. * For example, an array of string can be described as a Loadable. - * The {@link load} method + * The {@link load#emit} method */ - public sealed interface Loadable { + public sealed interface Loadable extends Snippet { /** * Generate the bytecode to load the Loadable onto the operand stack. * @param cob The CodeBuilder to add the bytecode for loading */ - void load(CodeBuilder cob); + @Override + void emit(CodeBuilder cob); /** * The type of the reference be loaded onto the operand stack. */ ClassDesc classDesc(); + } - /** - * Generate fields or methods needed to support the load of the Loadable. - * @param clb The ClassBuilder to setup the helpers. - */ - default void setup(ClassBuilder clb) {}; + public record Constant(T value) implements Snippet { + @Override + public void emit(CodeBuilder cob) { + cob.loadConstant(value); + } + } - /** - * Whether {@link setup} must be called to {@link load} properly. - */ - default boolean doesRequireSetup() { return false; } + public final record EnumConstant(Enum o) implements Loadable { + @Override + public void emit(CodeBuilder cob) { + cob.getstatic(classDesc(), o.name(), classDesc()); + } + + @Override + public ClassDesc classDesc() { + return o.getClass().describeConstable().get(); + } } /** @@ -92,15 +122,15 @@ public sealed interface Loadable { * @param isStatic Should the generated method be static or public * @throws IllegalArgumentException if the value is a {@code WrappedLoadable} */ - public record WrappedLoadable(Loadable value, ClassDesc ownerClass, String methodName, boolean isStatic) implements Loadable { - public WrappedLoadable { - if (value instanceof WrappedLoadable) { + public final record LoadableProvider(Loadable value, ClassDesc ownerClass, String methodName, boolean isStatic) implements Loadable { + public LoadableProvider { + if (value instanceof LoadableProvider) { throw new IllegalArgumentException(); } } @Override - public void load(CodeBuilder cob) { + public void emit(CodeBuilder cob) { if (isStatic()) { cob.invokestatic(ownerClass, methodName, methodType()); } else { @@ -114,12 +144,11 @@ public void setup(ClassBuilder clb) { // TODO: decide whether we should call value.setup(clb) // Prefer to have creator be responsible, given value // is provided to constructor, it should be ready to use. - clb.withMethodBody( - methodName, + clb.withMethodBody(methodName, methodType(), isStatic ? ACC_STATIC : ACC_PUBLIC, cob -> { - value.load(cob); + value.emit(cob); cob.areturn(); }); } @@ -129,11 +158,6 @@ public ClassDesc classDesc() { return value.classDesc(); } - @Override - public boolean doesRequireSetup() { - return true; - } - /** * Describe the method type of the generated provider method. */ @@ -142,45 +166,15 @@ public MethodTypeDesc methodType() { } } - public record LoadableEnum(Enum o) implements Loadable { - @Override - public void load(CodeBuilder cob) { - cob.getstatic(classDesc(), o.name(), classDesc()); - } - - @Override - public ClassDesc classDesc() { - return o.getClass().describeConstable().get(); - } - } - - /** - * A function to load an element of type {@code T} onto the operand stack. - * @param cob The {@link CodeBuilder} to generate load code. - * @param element The element to be load. - * @param index The index of the element in the containing collection. - */ - public interface ElementLoader { - void load(CodeBuilder cob, T element, int index); - - static ElementLoader of(BiConsumer ignoreIndex) { - return (cob, element, _) -> { - ignoreIndex.accept(cob, element); - }; - } - - @SuppressWarnings("unchecked") - static ElementLoader selfLoader() { - return (ElementLoader) LOADABLE_LOADER; - } - } - - /** - * Return a snippet builder that loads an enum onto the operand stack using - * the enum name static final field - */ - public static > ElementLoader getEnumLoader(ClassDesc enumClassDesc) { - return (cob, element, _) -> cob.getstatic(enumClassDesc, element.name(), enumClassDesc); + @FunctionalInterface + public interface CollectionElementBuilder { + /** + * Build a snippet to load the element onto the operand stack. + * @param element The element to be load. + * @param index The index of the element in the containing collection. + * @return A snippet of bytecodes to load the element onto the operand stack. + */ + Snippet build(T element, int index); } // Array supports @@ -192,8 +186,8 @@ public sealed interface LoadableArray extends Loadable { * * @param elementType The type of the array element * @param elements The elements for the array - * @param elementLoader The loader function to load a single element onto operand stack to - * be stored at given index + * @param elementLoader The snippet builder to generate bytecodes to load an element onto + * the operand stack * @param activatePagingThreshold Use pagination methods if the count of elements is larger * than the given value * @param ownerClassDesc The owner class for the paginattion methods @@ -205,7 +199,7 @@ public sealed interface LoadableArray extends Loadable { */ static LoadableArray of(ClassDesc elementType, Collection elements, - ElementLoader elementLoader, + CollectionElementBuilder elementLoader, int activatePagingThreshold, ClassDesc ownerClassDesc, String methodNamePrefix, @@ -223,13 +217,16 @@ static LoadableArray of(ClassDesc elementType, */ private sealed static abstract class AbstractLoadableArray implements LoadableArray { protected final ClassDesc elementType; - protected final Collection elements; - protected final ElementLoader elementLoader; + protected final ArrayList loadElementSnippets; - public AbstractLoadableArray(ClassDesc elementType, Collection elements, ElementLoader elementLoader) { + public AbstractLoadableArray(ClassDesc elementType, Collection elements, CollectionElementBuilder elementLoader) { this.elementType = elementType; - this.elements = elements; - this.elementLoader = elementLoader; + loadElementSnippets = new ArrayList<>(elements.size()); + for (var element: elements) { + loadElementSnippets.add(elementLoader.build(element, loadElementSnippets.size())); + } + + assert(loadElementSnippets.size() == elements.size()); } @Override @@ -237,13 +234,17 @@ public ClassDesc classDesc() { return elementType.arrayType(); } - protected void fill(CodeBuilder cob, Iterable elements, int offset) { - for (T t : elements) { + @Override + public void setup(ClassBuilder clb) { + loadElementSnippets.forEach(s -> s.setup(clb)); + } + + protected void fill(CodeBuilder cob, int fromIndex, int toIndex) { + for (var index = fromIndex; index < toIndex; index++) { cob.dup() // arrayref - .loadConstant(offset); - elementLoader.load(cob, t, offset); // value + .loadConstant(index); + loadElementSnippets.get(index).emit(cob); // value cob.aastore(); - offset++; } } } @@ -253,19 +254,19 @@ protected void fill(CodeBuilder cob, Iterable elements, int offset) { * new T[] { elements } */ public static final class SimpleArray extends AbstractLoadableArray { - public SimpleArray(ClassDesc elementType, T[] elements, ElementLoader elementLoader) { + public SimpleArray(ClassDesc elementType, T[] elements, CollectionElementBuilder elementLoader) { this(elementType, Arrays.asList(elements), elementLoader); } - public SimpleArray(ClassDesc elementType, Collection elements, ElementLoader elementLoader) { + public SimpleArray(ClassDesc elementType, Collection elements, CollectionElementBuilder elementLoader) { super(elementType, elements, elementLoader); } @Override - public void load(CodeBuilder cob) { - cob.loadConstant(elements.size()) + public void emit(CodeBuilder cob) { + cob.loadConstant(loadElementSnippets.size()) .anewarray(elementType); - fill(cob, elements, 0); + fill(cob, 0, loadElementSnippets.size()); } } @@ -298,7 +299,7 @@ public static final class PaginatedArray extends AbstractLoadableArray { public PaginatedArray(ClassDesc elementType, T[] elements, - ElementLoader elementLoader, + CollectionElementBuilder elementLoader, ClassDesc ownerClassDesc, String methodNamePrefix, int pageSize) { @@ -312,7 +313,7 @@ public PaginatedArray(ClassDesc elementType, public PaginatedArray(ClassDesc elementType, Collection elements, - ElementLoader elementLoader, + CollectionElementBuilder elementLoader, ClassDesc ownerClassDesc, String methodNamePrefix, int pageSize) { @@ -324,39 +325,36 @@ public PaginatedArray(ClassDesc elementType, } @Override - public void load(CodeBuilder cob) { + public void emit(CodeBuilder cob) { // Invoke the first page, which will call next page until fulfilled - cob.loadConstant(elements.size()) + cob.loadConstant(loadElementSnippets.size()) .anewarray(elementType) .invokestatic(ownerClassDesc, methodNamePrefix + "0", MTD_PageHelper); } + /** + * Generate helper methods to fill each page + */ @Override public void setup(ClassBuilder clb) { - var pages = paginate(elements, pageSize); - - assert(pages.size() == pageCount()); - - var lastPageNo = pages.size() - 1; + super.setup(clb); + var lastPageNo = pageCount() - 1; for (int pageNo = 0; pageNo <= lastPageNo; pageNo++) { - genFillPageHelper(clb, pages.get(pageNo), pageNo, pageNo < lastPageNo); + genFillPageHelper(clb, pageNo, pageNo < lastPageNo); } } - @Override - public boolean doesRequireSetup() { return true; } - // each helper function is T[] methodNamePrefix{pageNo}(T[]) // fill the page portion and chain calling to fill next page - private void genFillPageHelper(ClassBuilder clb, Collection pageElements, int pageNo, boolean hasNextPage) { - var offset = pageSize * pageNo; - clb.withMethodBody( - methodNamePrefix + pageNo, + private void genFillPageHelper(ClassBuilder clb, int pageNo, boolean hasNextPage) { + var fromIndex = pageSize * pageNo; + var toIndex = hasNextPage ? (fromIndex + pageSize) : loadElementSnippets.size(); + clb.withMethodBody(methodNamePrefix + pageNo, MTD_PageHelper, ACC_STATIC, mcob -> { mcob.aload(0); // arrayref - fill(mcob, pageElements, offset); + fill(mcob, fromIndex, toIndex); if (hasNextPage) { mcob.invokestatic( ownerClassDesc, @@ -368,11 +366,11 @@ private void genFillPageHelper(ClassBuilder clb, Collection pageElements, int } public boolean isLastPagePartial() { - return (elements.size() % pageSize) != 0; + return (loadElementSnippets.size() % pageSize) != 0; } public int pageCount() { - var pages = elements.size() / pageSize; + var pages = loadElementSnippets.size() / pageSize; return isLastPagePartial() ? pages + 1 : pages; } } @@ -382,7 +380,7 @@ public sealed interface LoadableSet extends Loadable { /** * Factory method for LoadableSet without using pagination methods. */ - static LoadableSet of(Collection elements, ElementLoader loader) { + static LoadableSet of(Collection elements, CollectionElementBuilder loader) { // Set::of implementation optimization with 2 elements if (elements.size() <= 2) { return new TinySet<>(elements, loader); @@ -396,7 +394,7 @@ static LoadableSet of(Collection elements, ElementLoader loader) { * given threshold. */ static LoadableSet of(Collection elements, - ElementLoader loader, + CollectionElementBuilder loader, int activatePagingThreshold, ClassDesc ownerClassDesc, String methodNamePrefix, @@ -422,28 +420,35 @@ default ClassDesc classDesc() { } private static final class TinySet implements LoadableSet { - Collection elements; - ElementLoader loader; + ArrayList loadElementSnippets; - TinySet(Collection elements, ElementLoader loader) { + TinySet(Collection elements, CollectionElementBuilder loader) { // The Set::of API supports up to 10 elements if (elements.size() > 10) { throw new IllegalArgumentException(); } - this.elements = elements; - this.loader = loader; + loadElementSnippets = new ArrayList<>(elements.size()); + for (T e: elements) { + loadElementSnippets.add(loader.build(e, loadElementSnippets.size())); + } } @Override - public void load(CodeBuilder cob) { - var index = 0; - for (T t : elements) { - loader.load(cob, t, index++); + public void emit(CodeBuilder cob) { + for (var snippet: loadElementSnippets) { + snippet.emit(cob); } - var mtdArgs = new ClassDesc[elements.size()]; + var mtdArgs = new ClassDesc[loadElementSnippets.size()]; Arrays.fill(mtdArgs, CD_Object); cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, mtdArgs), true); } + + @Override + public void setup(ClassBuilder clb) { + for (var snippet: loadElementSnippets) { + snippet.setup(clb); + } + } } private static final class ArrayAsSet implements LoadableSet { @@ -454,36 +459,14 @@ private static final class ArrayAsSet implements LoadableSet { } @Override - public void load(CodeBuilder cob) { - elements.load(cob); + public void emit(CodeBuilder cob) { + elements.emit(cob); cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, CD_Object.arrayType()), true); } - @Override - public boolean doesRequireSetup() { - return elements.doesRequireSetup(); - } - @Override public void setup(ClassBuilder clb) { elements.setup(clb); } } - - // utilities - private static ArrayList> paginate(Iterable elements, int pageSize) { - ArrayList> pages = new ArrayList<>(pageSize); - ArrayList currentPage = null; - var index = 0; - for (T element: elements) { - if (index % pageSize == 0) { - currentPage = new ArrayList<>(); - pages.add(currentPage); - } - currentPage.add(element); - index++; - } - - return pages; - } } \ No newline at end of file diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java similarity index 67% rename from src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java rename to src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java index d0e8a08caece2..33f4dc7f9bc85 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleInfoLoader.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2024, 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 @@ -39,21 +39,18 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Optional; import java.util.Set; -import java.util.function.Consumer; -import jdk.tools.jlink.internal.Snippets.ElementLoader; -import jdk.tools.jlink.internal.Snippets.Loadable; import jdk.tools.jlink.internal.Snippets.LoadableArray; import jdk.tools.jlink.internal.Snippets.LoadableSet; import static jdk.tools.jlink.internal.Snippets.STRING_LOADER; import static jdk.tools.jlink.internal.Snippets.STRING_PAGE_SIZE; +import jdk.tools.jlink.internal.Snippets.Snippet; import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.ModuleInfo; import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.SystemModulesClassGenerator.DedupSetBuilder; -class ModuleInfoLoader implements ElementLoader { +class ModuleDescriptorBuilder { private static final ClassDesc CD_MODULE_DESCRIPTOR = ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor"); private static final ClassDesc CD_MODULE_BUILDER = @@ -62,27 +59,17 @@ class ModuleInfoLoader implements ElementLoader { private static final int PAGING_THRESHOLD = 512; private final DedupSetBuilder dedupSetBuilder; private final ClassDesc ownerClassDesc; - private final ArrayList> amendments = new ArrayList<>(); - ModuleInfoLoader(DedupSetBuilder dedupSetBuilder, ClassDesc ownerClassDesc) { + ModuleDescriptorBuilder(DedupSetBuilder dedupSetBuilder, ClassDesc ownerClassDesc) { this.dedupSetBuilder = dedupSetBuilder; this.ownerClassDesc = ownerClassDesc; } - @Override - public void load(CodeBuilder cob, ModuleInfo moduleInfo, int index) { - var mdBuilder = new ModuleDescriptorBuilder(moduleInfo.descriptor(), moduleInfo.packages(), index); - mdBuilder.load(cob); - if (mdBuilder.doesRequireSetup()) { - amendments.add(mdBuilder::setup); - } - } - - public void finish(ClassBuilder clb) { - amendments.forEach(a -> a.accept(clb)); + public Snippet build(ModuleInfo moduleInfo, int index) { + return new ModuleDescriptorSnippet(moduleInfo.descriptor(), moduleInfo.packages(), index); } - class ModuleDescriptorBuilder { + class ModuleDescriptorSnippet implements Snippet { static final ClassDesc CD_EXPORTS = ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Exports"); static final ClassDesc CD_OPENS = @@ -126,40 +113,96 @@ class ModuleDescriptorBuilder { static final MethodTypeDesc MTD_ModuleDescriptor_int = MethodTypeDesc.of(CD_MODULE_DESCRIPTOR, CD_int); static final MethodTypeDesc MTD_List_ObjectArray = MethodTypeDesc.of(CD_List, CD_Object.arrayType()); - final ModuleDescriptor md; final Set packages; final int index; - Consumer amendment; + final Snippet requiresArray; + final Snippet exportsArray; + final Snippet opensArray; + final Snippet providesArray; + final Snippet packagesSet; - ModuleDescriptorBuilder(ModuleDescriptor md, Set packages, int index) { + ModuleDescriptorSnippet(ModuleDescriptor md, Set packages, int index) { if (md.isAutomatic()) { throw new InternalError("linking automatic module is not supported"); } - this.md = md; this.packages = packages; this.index = index; + requiresArray = buildRequiresArray(); + exportsArray = buildExportsArray(); + opensArray = buildOpensArray(); + providesArray = buildProvidesArray(); + packagesSet = buildPackagesSet(); + } + + /* + * Invoke Builder.newRequires(Set mods, String mn, String compiledVersion) + * + * Set mods = ... + * Builder.newRequires(mods, mn, compiledVersion); + */ + Snippet loadRequire(Requires require, int unused) { + return cob -> { + dedupSetBuilder.loadRequiresModifiers(cob, require.modifiers()); + cob.loadConstant(require.name()); + if (require.compiledVersion().isPresent()) { + cob.loadConstant(require.compiledVersion().get().toString()) + .invokestatic(CD_MODULE_BUILDER, + "newRequires", + MTD_REQUIRES_SET_STRING_STRING); + } else { + cob.invokestatic(CD_MODULE_BUILDER, + "newRequires", + MTD_REQUIRES_SET_STRING); + } + }; } - private LoadableArray requiresArray() { - var requiresArray = LoadableArray.of( + private LoadableArray buildRequiresArray() { + return LoadableArray.of( CD_REQUIRES, sorted(md.requires()), - this::newRequires, + this::loadRequire, PAGING_THRESHOLD, ownerClassDesc, "module" + index + "Requires", // number safe for a single page helper under 64K size limit 2000); + } - setupLoadable(requiresArray); - return requiresArray; + /* + * Invoke + * Builder.newExports(Set ms, String pn, + * Set targets) + * or + * Builder.newExports(Set ms, String pn) + * + * ms = export.modifiers() + * pn = export.source() + * targets = export.targets() + */ + Snippet loadExports(Exports export, int unused) { + return cob -> { + dedupSetBuilder.loadExportsModifiers(cob, export.modifiers()); + cob.loadConstant(export.source()); + var targets = export.targets(); + if (!targets.isEmpty()) { + dedupSetBuilder.loadStringSet(cob, targets); + cob.invokestatic(CD_MODULE_BUILDER, + "newExports", + MTD_EXPORTS_MODIFIER_SET_STRING_SET); + } else { + cob.invokestatic(CD_MODULE_BUILDER, + "newExports", + MTD_EXPORTS_MODIFIER_SET_STRING); + } + }; } - private LoadableArray exportArray() { - var exportArray = LoadableArray.of( + private LoadableArray buildExportsArray() { + return LoadableArray.of( CD_EXPORTS, sorted(md.exports()), this::loadExports, @@ -168,71 +211,113 @@ private LoadableArray exportArray() { "module" + index + "Exports", // number safe for a single page helper under 64K size limit 2000); + } - setupLoadable(exportArray); - return exportArray; + /* + * Invoke + * Builder.newOpens(Set ms, String pn, + * Set targets) + * or + * Builder.newOpens(Set ms, String pn) + * + * ms = open.modifiers() + * pn = open.source() + * targets = open.targets() + * Builder.newOpens(mods, pn, targets); + */ + Snippet loadOpens(Opens open, int unused) { + return cob -> { + dedupSetBuilder.loadOpensModifiers(cob, open.modifiers()); + cob.loadConstant(open.source()); + var targets = open.targets(); + if (!targets.isEmpty()) { + dedupSetBuilder.loadStringSet(cob, targets); + cob.invokestatic(CD_MODULE_BUILDER, + "newOpens", + MTD_OPENS_MODIFIER_SET_STRING_SET); + } else { + cob.invokestatic(CD_MODULE_BUILDER, + "newOpens", + MTD_OPENS_MODIFIER_SET_STRING); + } + }; } - private LoadableArray opensArray() { - var opensArray = LoadableArray.of( + private LoadableArray buildOpensArray() { + return LoadableArray.of( CD_OPENS, sorted(md.opens()), - this::newOpens, + this::loadOpens, PAGING_THRESHOLD, ownerClassDesc, "module" + index + "Opens", // number safe for a single page helper under 64K size limit 2000); + } - setupLoadable(opensArray); - return opensArray; + /* + * Invoke Builder.newProvides(String service, List providers) + * + * service = provide.service() + * providers = List.of(new String[] { provide.providers() } + * Builder.newProvides(service, providers); + */ + private Snippet loadProvides(Provides provide, int offset) { + return cob -> { + var providersArray = LoadableArray.of( + CD_String, + provide.providers(), + STRING_LOADER, + PAGING_THRESHOLD, + ownerClassDesc, + "module" + index + "Provider" + offset, + STRING_PAGE_SIZE); + + cob.loadConstant(provide.service()); + providersArray.emit(cob); + cob.invokestatic(CD_List, + "of", + MTD_List_ObjectArray, + true) + .invokestatic(CD_MODULE_BUILDER, + "newProvides", + MTD_PROVIDES_STRING_LIST); + }; } - private LoadableArray providesArray() { - var providesArray = LoadableArray.of( + private LoadableArray buildProvidesArray() { + return LoadableArray.of( CD_PROVIDES, sorted(md.provides()), - this::newProvides, + this::loadProvides, PAGING_THRESHOLD, ownerClassDesc, "module" + index + "Provides", // number safe for a single page helper under 64K size limit 2000); - - setupLoadable(providesArray); - return providesArray; } - private LoadableSet packagesSet() { - var packagesSet = LoadableSet.of( + private LoadableSet buildPackagesSet() { + return LoadableSet.of( sorted(packages), STRING_LOADER, PAGING_THRESHOLD, ownerClassDesc, "module" + index + "Packages", STRING_PAGE_SIZE); - - setupLoadable(packagesSet); - return packagesSet; - } - - private void setupLoadable(Loadable loadable) { - if (amendment == null) { - amendment = loadable::setup; - } else { - amendment = amendment.andThen(loadable::setup); - } - } - - boolean doesRequireSetup() { - return amendment != null; } - void setup(ClassBuilder clb) { - if (amendment != null) amendment.accept(clb); + @Override + public void setup(ClassBuilder clb) { + requiresArray.setup(clb); + exportsArray.setup(clb); + opensArray.setup(clb); + providesArray.setup(clb); + packagesSet.setup(clb); } - void load(CodeBuilder cob) { + @Override + public void emit(CodeBuilder cob) { // new jdk.internal.module.Builder cob.new_(CD_MODULE_BUILDER) .dup() @@ -251,19 +336,19 @@ void load(CodeBuilder cob) { } // requires - requiresArray().load(cob); + requiresArray.emit(cob); cob.invokevirtual(CD_MODULE_BUILDER, "requires", MTD_REQUIRES_ARRAY); // exports - exportArray().load(cob); + exportsArray.emit(cob); cob.invokevirtual(CD_MODULE_BUILDER, "exports", MTD_EXPORTS_ARRAY); // opens - opensArray().load(cob); + opensArray.emit(cob); cob.invokevirtual(CD_MODULE_BUILDER, "opens", MTD_OPENS_ARRAY); @@ -275,13 +360,13 @@ void load(CodeBuilder cob) { MTD_SET); // provides - providesArray().load(cob); + providesArray.emit(cob); cob.invokevirtual(CD_MODULE_BUILDER, "provides", MTD_PROVIDES_ARRAY); // all packages - packagesSet().load(cob); + packagesSet.emit(cob); cob.invokevirtual(CD_MODULE_BUILDER, "packages", MTD_SET); @@ -314,112 +399,6 @@ void setModuleProperty(CodeBuilder cob, String methodName, String value) { methodName, MTD_STRING); } - - /* - * Invoke Builder.newRequires(Set mods, String mn, String compiledVersion) - * - * Set mods = ... - * Builder.newRequires(mods, mn, compiledVersion); - */ - void newRequires(CodeBuilder cob, Requires require, int unused) { - dedupSetBuilder.loadRequiresModifiers(cob, require.modifiers()); - cob.loadConstant(require.name()); - if (require.compiledVersion().isPresent()) { - cob.loadConstant(require.compiledVersion().get().toString()) - .invokestatic(CD_MODULE_BUILDER, - "newRequires", - MTD_REQUIRES_SET_STRING_STRING); - } else { - cob.invokestatic(CD_MODULE_BUILDER, - "newRequires", - MTD_REQUIRES_SET_STRING); - } - } - - /* - * Invoke - * Builder.newExports(Set ms, String pn, - * Set targets) - * or - * Builder.newExports(Set ms, String pn) - * - * ms = export.modifiers() - * pn = export.source() - * targets = export.targets() - */ - void loadExports(CodeBuilder cb, Exports export, int unused) { - dedupSetBuilder.loadExportsModifiers(cb, export.modifiers()); - cb.loadConstant(export.source()); - var targets = export.targets(); - if (!targets.isEmpty()) { - dedupSetBuilder.loadStringSet(cb, targets); - cb.invokestatic(CD_MODULE_BUILDER, - "newExports", - MTD_EXPORTS_MODIFIER_SET_STRING_SET); - } else { - cb.invokestatic(CD_MODULE_BUILDER, - "newExports", - MTD_EXPORTS_MODIFIER_SET_STRING); - } - } - - /* - * Invoke - * Builder.newOpens(Set ms, String pn, - * Set targets) - * or - * Builder.newOpens(Set ms, String pn) - * - * ms = open.modifiers() - * pn = open.source() - * targets = open.targets() - * Builder.newOpens(mods, pn, targets); - */ - void newOpens(CodeBuilder cb, Opens open, int unused) { - dedupSetBuilder.loadOpensModifiers(cb, open.modifiers()); - cb.loadConstant(open.source()); - var targets = open.targets(); - if (!targets.isEmpty()) { - dedupSetBuilder.loadStringSet(cb, targets); - cb.invokestatic(CD_MODULE_BUILDER, - "newOpens", - MTD_OPENS_MODIFIER_SET_STRING_SET); - } else { - cb.invokestatic(CD_MODULE_BUILDER, - "newOpens", - MTD_OPENS_MODIFIER_SET_STRING); - } - } - - /* - * Invoke Builder.newProvides(String service, List providers) - * - * service = provide.service() - * providers = List.of(new String[] { provide.providers() } - * Builder.newProvides(service, providers); - */ - void newProvides(CodeBuilder cb, Provides provide, int offset) { - var providersArray = LoadableArray.of( - CD_String, - provide.providers(), - STRING_LOADER, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Provider" + offset, - STRING_PAGE_SIZE); - - setupLoadable(providersArray); - - cb.loadConstant(provide.service()); - providersArray.load(cb); - cb.invokestatic(CD_List, - "of", - MTD_List_ObjectArray, - true) - .invokestatic(CD_MODULE_BUILDER, - "newProvides", - MTD_PROVIDES_STRING_LIST); - } } /** @@ -430,7 +409,7 @@ void newProvides(CodeBuilder cb, Provides provide, int offset) { * @return a sorted copy of the given collection. */ private static > List sorted(Collection c) { - var l = new ArrayList(c); + var l = new ArrayList<>(c); Collections.sort(l); return l; } 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 41215b6a53c93..597b9ad65dba3 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 @@ -715,11 +715,10 @@ private void genIncubatorModules(ClassBuilder clb) { } private void genModuleDescriptorsMethod(ClassBuilder clb) { - var moduleInfoLoader = new ModuleInfoLoader(dedupSetBuilder, classDesc); - var moduleDescriptors = LoadableArray.of( - CD_MODULE_DESCRIPTOR, + var converter = new ModuleDescriptorBuilder(dedupSetBuilder, classDesc); + var moduleDescriptors = LoadableArray.of(CD_MODULE_DESCRIPTOR, moduleInfos, - moduleInfoLoader, + converter::build, moduleDescriptorsPerMethod, classDesc, "sub", @@ -734,12 +733,9 @@ private void genModuleDescriptorsMethod(ClassBuilder clb) { MTD_ModuleDescriptorArray, ACC_PUBLIC, cob -> { - moduleDescriptors.load(cob); - cob.areturn(); + moduleDescriptors.emit(cob); + cob.areturn(); }); - - // amend class with helpers needed by individual ModuleDescriptor - moduleInfoLoader.finish(clb); } /** @@ -948,7 +944,7 @@ private void generate(ClassBuilder clb, */ private void genImmutableSet(CodeBuilder cob, Set set) { var loadableSet = LoadableSet.of(sorted(set), STRING_LOADER); - loadableSet.load(cob); + loadableSet.emit(cob); } class ModuleHashesBuilder { @@ -1075,7 +1071,7 @@ static class DedupSetBuilder { this.owner = owner; } - > SetReference createLoadableSet(Set elements, ElementLoader elementLoader) { + > SetReference createLoadableSet(Set elements, CollectionElementBuilder elementLoader) { var loadableSet = LoadableSet.of(sorted(elements), elementLoader, PAGING_THRESHOLD, @@ -1102,7 +1098,7 @@ void stringSet(Set strings) { */ void exportsModifiers(Set mods) { exportsModifiersSets.computeIfAbsent(mods, - s -> createLoadableSet(s, getEnumLoader(CD_EXPORTS_MODIFIER)) + s -> createLoadableSet(s, (export, _) -> new EnumConstant(export)) ).increment(); } @@ -1111,7 +1107,7 @@ void exportsModifiers(Set mods) { */ void opensModifiers(Set mods) { opensModifiersSets.computeIfAbsent(mods, - s -> createLoadableSet(s, getEnumLoader(CD_OPENS_MODIFIER)) + s -> createLoadableSet(s, (open, _) -> new EnumConstant(open)) ).increment(); } @@ -1120,7 +1116,7 @@ void opensModifiers(Set mods) { */ void requiresModifiers(Set mods) { requiresModifiersSets.computeIfAbsent(mods, - s -> createLoadableSet(s, getEnumLoader(CD_REQUIRES_MODIFIER)) + s -> createLoadableSet(s, (require, _) -> new EnumConstant(require)) ).increment(); } @@ -1128,21 +1124,21 @@ void requiresModifiers(Set mods) { * Load the given set to the top of operand stack. */ void loadStringSet(CodeBuilder cob, Set names) { - stringSets.get(names).load(cob); + stringSets.get(names).emit(cob); } /* * Load the given set to the top of operand stack. */ void loadExportsModifiers(CodeBuilder cob, Set mods) { - exportsModifiersSets.get(mods).load(cob); + exportsModifiersSets.get(mods).emit(cob); } /* * Load the given set to the top of operand stack. */ void loadOpensModifiers(CodeBuilder cob, Set mods) { - opensModifiersSets.get(mods).load(cob); + opensModifiersSets.get(mods).emit(cob); } @@ -1150,13 +1146,13 @@ void loadOpensModifiers(CodeBuilder cob, Set mods) { * Load the given set to the top of operand stack. */ void loadRequiresModifiers(CodeBuilder cob, Set mods) { - requiresModifiersSets.get(mods).load(cob); + requiresModifiersSets.get(mods).emit(cob); } /* * Adding provider methods to the class. For those set used more than once, built * once and keep the reference for later access. - * Return a list of snippet to be used in . + * Return a snippet to setup the cache . * * The returned snippet would set up the set referenced more than once, * @@ -1174,7 +1170,7 @@ Optional> buildConstants(ClassBuilder clb) { var staticCache = new ArrayList(); for (var set: values) { - set.loadableSet().setup(clb); + set.setup(clb); if (set.refCount() > 1) { staticCache.add(set); } @@ -1184,28 +1180,20 @@ Optional> buildConstants(ClassBuilder clb) { return Optional.empty(); } - // This is called when the value is build for the cache - // At that time, a slot in cache is assigned - // The loader is called when building the static initializer - // We need to ensure that happens before we access SetReference::load - ElementLoader cacheLoader = (cob, setRef, index) -> { - setRef.assignTo(index); - setRef.loadableSet().load(cob); - }; - - var loadableArray = LoadableArray.of( + var cacheValuesArray = LoadableArray.of( CD_Set, staticCache, - cacheLoader, + SetReference::build, PAGING_THRESHOLD, owner, VALUES_ARRAY, 2000); - loadableArray.setup(clb); + cacheValuesArray.setup(clb); clb.withField(VALUES_ARRAY, CD_Set.arrayType(), ACC_STATIC | ACC_FINAL); + return Optional.of(cob -> { - loadableArray.load(cob); + cacheValuesArray.emit(cob); cob.putstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); }); } @@ -1217,7 +1205,7 @@ Optional> buildConstants(ClassBuilder clb) { * and load from there. Otherwise, the set is built in place and load onto the operand * stack. */ - class SetReference { + class SetReference implements Snippet { // sorted elements of the set to ensure same generated code private final LoadableSet loadableSet; @@ -1237,29 +1225,38 @@ int refCount() { return refCount; } - LoadableSet loadableSet() { - return loadableSet; - } - - void assignTo(int index) { - this.index = index; - } - // Load the set to the operand stack. // When referenced more than once, the value is pre-built with static initialzer // and is load from the cache array with // dedupSetValues[index] // Otherwise, LoadableSet will load the set onto the operand stack. - void load(CodeBuilder cob) { + @Override + public void emit(CodeBuilder cob) { if (refCount > 1) { assert index >= 0; cob.getstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); cob.loadConstant(index); cob.aaload(); } else { - loadableSet.load(cob); + loadableSet.emit(cob); } } + + @Override + public void setup(ClassBuilder clb) { + loadableSet.setup(clb); + } + + /** + * Build the snippet to load the set onto the operand stack for storing into cache + * to be later accessed by the SetReference::emit + */ + Snippet build(int index) { + return cob -> { + this.index = index; + loadableSet.emit(cob); + }; + } } } } diff --git a/test/jdk/tools/jlink/SnippetsTest.java b/test/jdk/tools/jlink/SnippetsTest.java index e42c8d165efe7..5bc83a7b0f4cd 100644 --- a/test/jdk/tools/jlink/SnippetsTest.java +++ b/test/jdk/tools/jlink/SnippetsTest.java @@ -50,7 +50,6 @@ import jdk.tools.jlink.internal.Snippets.*; import static jdk.tools.jlink.internal.Snippets.*; -import jdk.tools.jlink.internal.Snippets.ElementLoader; /* * @test @@ -92,18 +91,9 @@ void testStringArrayLimitsWithoutPagination() { } } - @Test - void testEnumLoader() { - ClassDesc CD_ACCESSFLAG = AccessFlag.class.describeConstable().get(); - var expected = AccessFlag.values(); - var loadable = new SimpleArray(CD_ACCESSFLAG, expected, getEnumLoader(CD_ACCESSFLAG)); - Supplier supplier = generateSupplier("TestEnumLoader", loadable); - assertArrayEquals(expected, supplier.get()); - } - @ParameterizedTest @ValueSource(booleans = { true, false }) - void testWrapperLoadable(boolean isStatic) throws NoSuchMethodException { + void testLoadableProvider(boolean isStatic) throws NoSuchMethodException { var expected = IntStream.range(0, 1234) .mapToObj(i -> "WrapperTestString" + i) .toList(); @@ -116,20 +106,20 @@ void testWrapperLoadable(boolean isStatic) throws NoSuchMethodException { assertEquals(13, loadable.pageCount()); assertTrue(loadable.isLastPagePartial()); - var wrapped = new WrappedLoadable(loadable, testClassDesc, "wrapper", isStatic); - Supplier supplier = generateSupplier(className, wrapped, loadable); + var provider = new LoadableProvider(loadable, testClassDesc, "wrapper", isStatic); + Supplier supplier = generateSupplier(className, provider, loadable); verifyPaginationMethods(supplier.getClass(), String.class, "page", 13); assertArrayEquals(expected.toArray(), supplier.get()); // check wrapper function var methodType = MethodType.methodType(String[].class); try { - lookup().findStatic(supplier.getClass(), wrapped.methodName(), methodType); + lookup().findStatic(supplier.getClass(), provider.methodName(), methodType); } catch (IllegalAccessException ex) { assertFalse(isStatic); } try { - lookup().findVirtual(supplier.getClass(), wrapped.methodName(), methodType); + lookup().findVirtual(supplier.getClass(), provider.methodName(), methodType); } catch (IllegalAccessException ex) { assertTrue(isStatic); } @@ -144,10 +134,10 @@ void testLoadableEnum() { ModuleDescriptor.Requires.Modifier.TRANSITIVE }; - var loadable = new SimpleArray( + var loadable = new SimpleArray( Enum.class.describeConstable().get(), - Arrays.stream(enums).map(LoadableEnum::new).toList(), - ElementLoader.selfLoader()); + Arrays.stream(enums).map(EnumConstant::new).toList(), + (enumConstant, _) -> enumConstant); Supplier[]> supplier = generateSupplier("LoadableEnumTest", loadable); assertArrayEquals(enums, supplier.get()); @@ -280,9 +270,7 @@ byte[] generateSupplierClass(ClassDesc testClassDesc, Loadable loadable, Loadabl cob.return_(); }); - if (loadable.doesRequireSetup()) { - loadable.setup(clb); - } + loadable.setup(clb); for (var e: extra) { // always call setup should be no harm @@ -291,7 +279,7 @@ byte[] generateSupplierClass(ClassDesc testClassDesc, Loadable loadable, Loadabl } clb.withMethodBody("get", MethodTypeDesc.of(CD_Object), ACC_PUBLIC, cob -> { - loadable.load(cob); + loadable.emit(cob); cob.areturn(); }); }); From 784fd41c53c886711c94ce6126cf0f68d086a8fd Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Fri, 15 Nov 2024 09:32:49 -0800 Subject: [PATCH 11/15] Minor cleanup --- .../share/classes/jdk/tools/jlink/internal/Snippets.java | 8 ++++---- .../jlink/internal/plugins/ModuleDescriptorBuilder.java | 6 ++---- .../tools/jlink/internal/plugins/SystemModulesPlugin.java | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java index 91acea0dde2ff..296684a9701e4 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -169,10 +169,10 @@ public MethodTypeDesc methodType() { @FunctionalInterface public interface CollectionElementBuilder { /** - * Build a snippet to load the element onto the operand stack. - * @param element The element to be load. - * @param index The index of the element in the containing collection. - * @return A snippet of bytecodes to load the element onto the operand stack. + * Build a snippet for the element at the index. + * @param element The element + * @param index The index of the element in the containing collection + * @return A snippet of bytecodes to process the element */ Snippet build(T element, int index); } diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java index 33f4dc7f9bc85..5082c4b45b85e 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java @@ -114,7 +114,6 @@ class ModuleDescriptorSnippet implements Snippet { static final MethodTypeDesc MTD_List_ObjectArray = MethodTypeDesc.of(CD_List, CD_Object.arrayType()); final ModuleDescriptor md; - final Set packages; final int index; final Snippet requiresArray; final Snippet exportsArray; @@ -128,13 +127,12 @@ class ModuleDescriptorSnippet implements Snippet { } this.md = md; - this.packages = packages; this.index = index; requiresArray = buildRequiresArray(); exportsArray = buildExportsArray(); opensArray = buildOpensArray(); providesArray = buildProvidesArray(); - packagesSet = buildPackagesSet(); + packagesSet = buildPackagesSet(packages); } /* @@ -297,7 +295,7 @@ private LoadableArray buildProvidesArray() { 2000); } - private LoadableSet buildPackagesSet() { + private LoadableSet buildPackagesSet(Collection packages) { return LoadableSet.of( sorted(packages), STRING_LOADER, 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 597b9ad65dba3..483e13814e992 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 @@ -1252,8 +1252,8 @@ public void setup(ClassBuilder clb) { * to be later accessed by the SetReference::emit */ Snippet build(int index) { + this.index = index; return cob -> { - this.index = index; loadableSet.emit(cob); }; } From 9dbea0480e27b541bb2a7bfb81c0c2def84d7c07 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Fri, 13 Dec 2024 11:26:00 -0800 Subject: [PATCH 12/15] Move up Snippet setup as a builder --- .../jdk/tools/jlink/internal/Snippets.java | 435 ++++++------------ .../plugins/ModuleDescriptorBuilder.java | 166 +++---- .../internal/plugins/SystemModulesPlugin.java | 320 ++++++------- test/jdk/tools/jlink/SnippetsTest.java | 162 +++---- 4 files changed, 430 insertions(+), 653 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java index 296684a9701e4..a22e2d93816ce 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 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 @@ -27,32 +27,25 @@ import java.lang.classfile.ClassBuilder; import java.lang.classfile.CodeBuilder; import java.lang.constant.ClassDesc; -import java.lang.constant.ConstantDesc; import java.lang.constant.MethodTypeDesc; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Objects; -import static java.lang.classfile.ClassFile.ACC_PUBLIC; import static java.lang.classfile.ClassFile.ACC_STATIC; +import java.lang.constant.ConstantDesc; import static java.lang.constant.ConstantDescs.CD_Integer; import static java.lang.constant.ConstantDescs.CD_Object; import static java.lang.constant.ConstantDescs.CD_Set; import static java.lang.constant.ConstantDescs.CD_int; +import java.util.function.Function; + public class Snippets { // Tested page size of string array public static final int STRING_PAGE_SIZE = 8000; - - public static final CollectionElementBuilder STRING_LOADER = (value, index) -> new Constant<>(value); - - public static final CollectionElementBuilder INTEGER_LOADER = (value, index) -> cob -> { - // loadConstant will unbox - cob.loadConstant(value) - .invokestatic(ClassDesc.ofInternalName("java/lang/Integer"), "valueOf", MethodTypeDesc.of(CD_Integer, CD_int)); - }; - - public static final CollectionElementBuilder LOADABLE_LOADER = (loadable, index) -> loadable; - + // Tested page size of enum array + public static final int ENUM_PAGE_SIZE = 5000; /** * Snippet of bytecodes */ @@ -66,208 +59,137 @@ public interface Snippet { */ void emit(CodeBuilder cob); - /** - * Perpare a snippet needs some extra support like field or methods from the class. - * - * @param clb The ClassBuilder to setup the helpers. - */ - default void setup(ClassBuilder clb) {}; + static Snippet loadConstant(T v) { + return cob -> cob.loadConstant(v); + } + + static Snippet loadEnum(Enum e) { + var classDesc = e.getClass().describeConstable().get(); + return cob -> cob.getstatic(classDesc, e.name(), classDesc); + } + + static Snippet loadInteger(int value) { + return cob -> + cob.loadConstant(value) + .invokestatic(ClassDesc.ofInternalName("java/lang/Integer"), "valueOf", MethodTypeDesc.of(CD_Integer, CD_int)); + } + + static Snippet[] buildAll(Collection elements, Function fn) { + return elements.stream() + .map(fn) + .toArray(Snippet[]::new); + } } /** * Describe a reference that can be load onto the operand stack. * For example, an array of string can be described as a Loadable. - * The {@link load#emit} method */ - public sealed interface Loadable extends Snippet { + public record Loadable(ClassDesc classDesc, Snippet load) implements Snippet { /** * Generate the bytecode to load the Loadable onto the operand stack. * @param cob The CodeBuilder to add the bytecode for loading */ @Override - void emit(CodeBuilder cob); - - /** - * The type of the reference be loaded onto the operand stack. - */ - ClassDesc classDesc(); - } - - public record Constant(T value) implements Snippet { - @Override public void emit(CodeBuilder cob) { - cob.loadConstant(value); + load.emit(cob); } } - public final record EnumConstant(Enum o) implements Loadable { - @Override - public void emit(CodeBuilder cob) { - cob.getstatic(classDesc(), o.name(), classDesc()); - } + @FunctionalInterface + public interface IndexedElementSnippetBuilder { + /** + * Build a snippet for the element at the index. + * @param element The element + * @param index The index of the element in the containing collection + * @return A snippet of bytecodes to process the element + */ + Snippet build(T element, int index); - @Override - public ClassDesc classDesc() { - return o.getClass().describeConstable().get(); + default Snippet[] buildAll(Collection elements) { + var loadElementSnippets = new ArrayList(elements.size()); + for (var element: elements) { + loadElementSnippets.add(build(element, loadElementSnippets.size())); + } + + assert(loadElementSnippets.size() == elements.size()); + return loadElementSnippets.toArray(Snippet[]::new); } } - /** - * Generate a provider method for the {@code Loadable}. The provided - * Loadable should be ready for load. The caller is responsible to ensure - * the given Loadable had being setup properly. - * @param value The actuall {@code Loadable} to be wrapped into a method - * @param ownerClass The class of the generated method - * @param methodName The method name - * @param isStatic Should the generated method be static or public - * @throws IllegalArgumentException if the value is a {@code WrappedLoadable} - */ - public final record LoadableProvider(Loadable value, ClassDesc ownerClass, String methodName, boolean isStatic) implements Loadable { - public LoadableProvider { - if (value instanceof LoadableProvider) { - throw new IllegalArgumentException(); - } + public static record PagingContext(int total, int pageSize) { + public boolean isLastPagePartial() { + return (total % pageSize) != 0; } - @Override - public void emit(CodeBuilder cob) { - if (isStatic()) { - cob.invokestatic(ownerClass, methodName, methodType()); - } else { - cob.aload(0) - .invokevirtual(ownerClass, methodName, methodType()); - } + public int pageCount() { + var pages = total / pageSize; + return isLastPagePartial() ? pages + 1 : pages; } - @Override - public void setup(ClassBuilder clb) { - // TODO: decide whether we should call value.setup(clb) - // Prefer to have creator be responsible, given value - // is provided to constructor, it should be ready to use. - clb.withMethodBody(methodName, - methodType(), - isStatic ? ACC_STATIC : ACC_PUBLIC, - cob -> { - value.emit(cob); - cob.areturn(); - }); + public int lastPageSize() { + if (total == 0) return 0; + var remaining = total % pageSize; + return remaining == 0 ? pageSize : remaining; } + } - @Override - public ClassDesc classDesc() { - return value.classDesc(); + public static abstract class CollectionSnippetBuilder { + protected ClassDesc elementType; + protected int activatePagingThreshold; + protected ClassDesc ownerClassDesc; + protected String methodNamePrefix; + protected int pageSize; + protected ClassBuilder clb; + + protected CollectionSnippetBuilder(ClassDesc elementType) { + this.elementType = Objects.requireNonNull(elementType); } /** - * Describe the method type of the generated provider method. + * @param activatePagingThreshold Use pagination methods if the count of elements is larger + * than the given value */ - public MethodTypeDesc methodType() { - return MethodTypeDesc.of(classDesc()); + public CollectionSnippetBuilder activatePagingThreshold(int activatePagingThreshold) { + this.activatePagingThreshold = activatePagingThreshold; + return this; } - } - @FunctionalInterface - public interface CollectionElementBuilder { /** - * Build a snippet for the element at the index. - * @param element The element - * @param index The index of the element in the containing collection - * @return A snippet of bytecodes to process the element + * @param ownerClassDesc The owner class for the paginattion methods */ - Snippet build(T element, int index); - } + public CollectionSnippetBuilder ownerClassDesc(ClassDesc ownerClassDesc) { + this.ownerClassDesc = ownerClassDesc; + return this; + } - // Array supports - public sealed interface LoadableArray extends Loadable { /** - * Factory method to create a LoadableArray. - * The bytecode generated varies based on the number of elements and can have supporting - * methods for pagination, helps to overcome the code size limitation. - * - * @param elementType The type of the array element - * @param elements The elements for the array - * @param elementLoader The snippet builder to generate bytecodes to load an element onto - * the operand stack - * @param activatePagingThreshold Use pagination methods if the count of elements is larger - * than the given value - * @param ownerClassDesc The owner class for the paginattion methods * @param methodNamePrefix The method name prefix. Generated method will have the name of * this value appended with page number - * @param pageSize The count of elements per page - * - * @return A LoadableArray */ - static LoadableArray of(ClassDesc elementType, - Collection elements, - CollectionElementBuilder elementLoader, - int activatePagingThreshold, - ClassDesc ownerClassDesc, - String methodNamePrefix, - int pageSize) { - if (elements.size() > activatePagingThreshold) { - return new PaginatedArray<>(elementType, elements, elementLoader, ownerClassDesc, methodNamePrefix, pageSize); - } else { - return new SimpleArray<>(elementType, elements, elementLoader); - } - } - } - - /** - * Base class for all LoadableArray implementation. - */ - private sealed static abstract class AbstractLoadableArray implements LoadableArray { - protected final ClassDesc elementType; - protected final ArrayList loadElementSnippets; - - public AbstractLoadableArray(ClassDesc elementType, Collection elements, CollectionElementBuilder elementLoader) { - this.elementType = elementType; - loadElementSnippets = new ArrayList<>(elements.size()); - for (var element: elements) { - loadElementSnippets.add(elementLoader.build(element, loadElementSnippets.size())); - } - - assert(loadElementSnippets.size() == elements.size()); - } - - @Override - public ClassDesc classDesc() { - return elementType.arrayType(); + public CollectionSnippetBuilder methodNamePrefix(String methodNamePrefix) { + this.methodNamePrefix = methodNamePrefix; + return this; } - @Override - public void setup(ClassBuilder clb) { - loadElementSnippets.forEach(s -> s.setup(clb)); + /** + * @param pageSize The count of elements per page* + */ + public CollectionSnippetBuilder pageSize(int pageSize) { + this.pageSize = pageSize; + return this; } - protected void fill(CodeBuilder cob, int fromIndex, int toIndex) { - for (var index = fromIndex; index < toIndex; index++) { - cob.dup() // arrayref - .loadConstant(index); - loadElementSnippets.get(index).emit(cob); // value - cob.aastore(); - } + public CollectionSnippetBuilder classBuilder(ClassBuilder clb) { + this.clb = clb; + return this; } - } - /** - * Generate bytecode to create an array and assign values inline. Effectively as - * new T[] { elements } - */ - public static final class SimpleArray extends AbstractLoadableArray { - public SimpleArray(ClassDesc elementType, T[] elements, CollectionElementBuilder elementLoader) { - this(elementType, Arrays.asList(elements), elementLoader); + protected boolean shouldPaginate(int length) { + return activatePagingThreshold != 0 && length > activatePagingThreshold; } - public SimpleArray(ClassDesc elementType, Collection elements, CollectionElementBuilder elementLoader) { - super(elementType, elements, elementLoader); - } - - @Override - public void emit(CodeBuilder cob) { - cob.loadConstant(loadElementSnippets.size()) - .anewarray(elementType); - fill(cob, 0, loadElementSnippets.size()); - } + abstract public Loadable build(Snippet[] loadElementSnippets); } /** @@ -291,64 +213,55 @@ public void emit(CodeBuilder cob) { * } * and the last page will stop the chain and can be partial instead of full page size. */ - public static final class PaginatedArray extends AbstractLoadableArray { - final int pageSize; - final ClassDesc ownerClassDesc; - final String methodNamePrefix; + public static class ArraySnippetBuilder extends CollectionSnippetBuilder { final MethodTypeDesc MTD_PageHelper; + final ClassDesc classDesc; + Snippet[] loadElementSnippets; - public PaginatedArray(ClassDesc elementType, - T[] elements, - CollectionElementBuilder elementLoader, - ClassDesc ownerClassDesc, - String methodNamePrefix, - int pageSize) { - this(elementType, - Arrays.asList(elements), - elementLoader, - ownerClassDesc, - methodNamePrefix, - pageSize); + public ArraySnippetBuilder(ClassDesc elementType) { + super(elementType); + classDesc = elementType.arrayType(); + MTD_PageHelper = MethodTypeDesc.of(classDesc, classDesc); } - public PaginatedArray(ClassDesc elementType, - Collection elements, - CollectionElementBuilder elementLoader, - ClassDesc ownerClassDesc, - String methodNamePrefix, - int pageSize) { - super(elementType, elements, elementLoader); - this.ownerClassDesc = ownerClassDesc; - this.methodNamePrefix = methodNamePrefix; - this.pageSize = pageSize; - MTD_PageHelper = MethodTypeDesc.of(classDesc(), classDesc()); + protected void fill(CodeBuilder cob, int fromIndex, int toIndex) { + for (var index = fromIndex; index < toIndex; index++) { + cob.dup() // arrayref + .loadConstant(index); + loadElementSnippets[index].emit(cob); // value + cob.aastore(); + } } - @Override - public void emit(CodeBuilder cob) { + private void invokePageHelper(CodeBuilder cob) { // Invoke the first page, which will call next page until fulfilled - cob.loadConstant(loadElementSnippets.size()) + cob.loadConstant(loadElementSnippets.length) .anewarray(elementType) .invokestatic(ownerClassDesc, methodNamePrefix + "0", MTD_PageHelper); } + private void newArray(CodeBuilder cob) { + cob.loadConstant(loadElementSnippets.length) + .anewarray(elementType); + fill(cob, 0, loadElementSnippets.length); + } + /** * Generate helper methods to fill each page */ - @Override - public void setup(ClassBuilder clb) { - super.setup(clb); - var lastPageNo = pageCount() - 1; + private void setupHelpers() { + Objects.requireNonNull(clb); + var lastPageNo = new PagingContext(loadElementSnippets.length, pageSize).pageCount() - 1; for (int pageNo = 0; pageNo <= lastPageNo; pageNo++) { - genFillPageHelper(clb, pageNo, pageNo < lastPageNo); + genFillPageHelper(pageNo, pageNo < lastPageNo); } } // each helper function is T[] methodNamePrefix{pageNo}(T[]) // fill the page portion and chain calling to fill next page - private void genFillPageHelper(ClassBuilder clb, int pageNo, boolean hasNextPage) { + private void genFillPageHelper(int pageNo, boolean hasNextPage) { var fromIndex = pageSize * pageNo; - var toIndex = hasNextPage ? (fromIndex + pageSize) : loadElementSnippets.size(); + var toIndex = hasNextPage ? (fromIndex + pageSize) : loadElementSnippets.length; clb.withMethodBody(methodNamePrefix + pageNo, MTD_PageHelper, ACC_STATIC, @@ -365,108 +278,46 @@ private void genFillPageHelper(ClassBuilder clb, int pageNo, boolean hasNextPage }); } - public boolean isLastPagePartial() { - return (loadElementSnippets.size() % pageSize) != 0; - } - - public int pageCount() { - var pages = loadElementSnippets.size() / pageSize; - return isLastPagePartial() ? pages + 1 : pages; - } - } - - // Set support - public sealed interface LoadableSet extends Loadable { - /** - * Factory method for LoadableSet without using pagination methods. - */ - static LoadableSet of(Collection elements, CollectionElementBuilder loader) { - // Set::of implementation optimization with 2 elements - if (elements.size() <= 2) { - return new TinySet<>(elements, loader); - } else { - return new ArrayAsSet<>(new SimpleArray<>(CD_Object, elements, loader)); - } - } - /** - * Factory method for LoadableSet pagination methods when element count is larger than - * given threshold. - */ - static LoadableSet of(Collection elements, - CollectionElementBuilder loader, - int activatePagingThreshold, - ClassDesc ownerClassDesc, - String methodNamePrefix, - int pageSize) { - if (elements.size() > activatePagingThreshold) { - return new ArrayAsSet<>(LoadableArray.of( - CD_Object, - elements, - loader, - activatePagingThreshold, - ownerClassDesc, - methodNamePrefix, - pageSize)); + @Override + public Loadable build(Snippet[] loadElementSnippets) { + this.loadElementSnippets = Objects.requireNonNull(loadElementSnippets); + if (shouldPaginate(loadElementSnippets.length)) { + setupHelpers(); + return new Loadable(classDesc, this::invokePageHelper); } else { - return LoadableSet.of(elements, loader); + return new Loadable(classDesc, this::newArray); } } - - @Override - default ClassDesc classDesc() { - return CD_Set; - } } - private static final class TinySet implements LoadableSet { - ArrayList loadElementSnippets; - - TinySet(Collection elements, CollectionElementBuilder loader) { - // The Set::of API supports up to 10 elements - if (elements.size() > 10) { - throw new IllegalArgumentException(); - } - loadElementSnippets = new ArrayList<>(elements.size()); - for (T e: elements) { - loadElementSnippets.add(loader.build(e, loadElementSnippets.size())); - } + // Set support + public static class SetSnippetBuilder extends ArraySnippetBuilder { + public SetSnippetBuilder(ClassDesc elementType) { + super(elementType); } - @Override - public void emit(CodeBuilder cob) { + private void buildTinySet(CodeBuilder cob) { for (var snippet: loadElementSnippets) { snippet.emit(cob); } - var mtdArgs = new ClassDesc[loadElementSnippets.size()]; + var mtdArgs = new ClassDesc[loadElementSnippets.length]; Arrays.fill(mtdArgs, CD_Object); cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, mtdArgs), true); } @Override - public void setup(ClassBuilder clb) { - for (var snippet: loadElementSnippets) { - snippet.setup(clb); + public Loadable build(Snippet[] loadElementSnippets) { + if (loadElementSnippets.length <= 2) { + this.loadElementSnippets = loadElementSnippets; + return new Loadable(CD_Set, this::buildTinySet); + } else { + var array = super.build(loadElementSnippets); + return new Loadable(CD_Set, cob -> { + array.emit(cob); + cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, CD_Object.arrayType()), true); + }); } } } - - private static final class ArrayAsSet implements LoadableSet { - final LoadableArray elements; - - ArrayAsSet(LoadableArray elements) { - this.elements = elements; - } - - @Override - public void emit(CodeBuilder cob) { - elements.emit(cob); - cob.invokestatic(CD_Set, "of", MethodTypeDesc.of(CD_Set, CD_Object.arrayType()), true); - } - - @Override - public void setup(ClassBuilder clb) { - elements.setup(clb); - } - } } \ No newline at end of file diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java index 5082c4b45b85e..6efa4f49be509 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java @@ -41,32 +41,31 @@ import java.util.List; import java.util.Set; -import jdk.tools.jlink.internal.Snippets.LoadableArray; -import jdk.tools.jlink.internal.Snippets.LoadableSet; -import static jdk.tools.jlink.internal.Snippets.STRING_LOADER; -import static jdk.tools.jlink.internal.Snippets.STRING_PAGE_SIZE; -import jdk.tools.jlink.internal.Snippets.Snippet; +import static jdk.tools.jlink.internal.Snippets.*; import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.ModuleInfo; -import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.SystemModulesClassGenerator.DedupSetBuilder; +import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.SystemModulesClassGenerator.DedupSet; -class ModuleDescriptorBuilder { +class ModuleDescriptorBuilder implements IndexedElementSnippetBuilder { private static final ClassDesc CD_MODULE_DESCRIPTOR = ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor"); private static final ClassDesc CD_MODULE_BUILDER = ClassDesc.ofInternalName("jdk/internal/module/Builder"); private static final int PAGING_THRESHOLD = 512; - private final DedupSetBuilder dedupSetBuilder; + private final DedupSet dedupSet; private final ClassDesc ownerClassDesc; + private final ClassBuilder clb; - ModuleDescriptorBuilder(DedupSetBuilder dedupSetBuilder, ClassDesc ownerClassDesc) { - this.dedupSetBuilder = dedupSetBuilder; + ModuleDescriptorBuilder(ClassBuilder clb, DedupSet dedupSet, ClassDesc ownerClassDesc) { + this.clb = clb; + this.dedupSet = dedupSet; this.ownerClassDesc = ownerClassDesc; } + @Override public Snippet build(ModuleInfo moduleInfo, int index) { - return new ModuleDescriptorSnippet(moduleInfo.descriptor(), moduleInfo.packages(), index); + return new ModuleDescriptorSnippet(clb, moduleInfo.descriptor(), moduleInfo.packages(), index); } class ModuleDescriptorSnippet implements Snippet { @@ -121,18 +120,18 @@ class ModuleDescriptorSnippet implements Snippet { final Snippet providesArray; final Snippet packagesSet; - ModuleDescriptorSnippet(ModuleDescriptor md, Set packages, int index) { + ModuleDescriptorSnippet(ClassBuilder clb, ModuleDescriptor md, Set packages, int index) { if (md.isAutomatic()) { throw new InternalError("linking automatic module is not supported"); } this.md = md; this.index = index; - requiresArray = buildRequiresArray(); - exportsArray = buildExportsArray(); - opensArray = buildOpensArray(); - providesArray = buildProvidesArray(); - packagesSet = buildPackagesSet(packages); + requiresArray = buildRequiresArray(clb); + exportsArray = buildExportsArray(clb); + opensArray = buildOpensArray(clb); + providesArray = buildProvidesArray(clb); + packagesSet = buildPackagesSet(clb, packages); } /* @@ -141,9 +140,9 @@ class ModuleDescriptorSnippet implements Snippet { * Set mods = ... * Builder.newRequires(mods, mn, compiledVersion); */ - Snippet loadRequire(Requires require, int unused) { + Snippet loadRequire(Requires require) { return cob -> { - dedupSetBuilder.loadRequiresModifiers(cob, require.modifiers()); + dedupSet.requiresModifiersSets().get(require.modifiers()).emit(cob); cob.loadConstant(require.name()); if (require.compiledVersion().isPresent()) { cob.loadConstant(require.compiledVersion().get().toString()) @@ -158,16 +157,14 @@ Snippet loadRequire(Requires require, int unused) { }; } - private LoadableArray buildRequiresArray() { - return LoadableArray.of( - CD_REQUIRES, - sorted(md.requires()), - this::loadRequire, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Requires", - // number safe for a single page helper under 64K size limit - 2000); + private Snippet buildRequiresArray(ClassBuilder clb) { + return new ArraySnippetBuilder(CD_REQUIRES) + .classBuilder(clb) + .activatePagingThreshold(PAGING_THRESHOLD) + .ownerClassDesc(ownerClassDesc) + .methodNamePrefix("module" + index + "Requires") + .pageSize(2000) // number safe for a single page helper under 64K size limit + .build(Snippet.buildAll(sorted(md.requires()), this::loadRequire)); } /* @@ -181,13 +178,13 @@ private LoadableArray buildRequiresArray() { * pn = export.source() * targets = export.targets() */ - Snippet loadExports(Exports export, int unused) { + Snippet loadExports(Exports export) { return cob -> { - dedupSetBuilder.loadExportsModifiers(cob, export.modifiers()); + dedupSet.exportsModifiersSets().get(export.modifiers()).emit(cob); cob.loadConstant(export.source()); var targets = export.targets(); if (!targets.isEmpty()) { - dedupSetBuilder.loadStringSet(cob, targets); + dedupSet.stringSets().get(targets).emit(cob); cob.invokestatic(CD_MODULE_BUILDER, "newExports", MTD_EXPORTS_MODIFIER_SET_STRING_SET); @@ -199,16 +196,14 @@ Snippet loadExports(Exports export, int unused) { }; } - private LoadableArray buildExportsArray() { - return LoadableArray.of( - CD_EXPORTS, - sorted(md.exports()), - this::loadExports, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Exports", - // number safe for a single page helper under 64K size limit - 2000); + private Snippet buildExportsArray(ClassBuilder clb) { + return new ArraySnippetBuilder(CD_EXPORTS) + .classBuilder(clb) + .activatePagingThreshold(PAGING_THRESHOLD) + .ownerClassDesc(ownerClassDesc) + .methodNamePrefix("module" + index + "Exports") + .pageSize(2000) // number safe for a single page helper under 64K size limit + .build(Snippet.buildAll(sorted(md.exports()), this::loadExports)); } /* @@ -223,13 +218,13 @@ private LoadableArray buildExportsArray() { * targets = open.targets() * Builder.newOpens(mods, pn, targets); */ - Snippet loadOpens(Opens open, int unused) { + Snippet loadOpens(Opens open) { return cob -> { - dedupSetBuilder.loadOpensModifiers(cob, open.modifiers()); + dedupSet.opensModifiersSets().get(open.modifiers()).emit(cob); cob.loadConstant(open.source()); var targets = open.targets(); if (!targets.isEmpty()) { - dedupSetBuilder.loadStringSet(cob, targets); + dedupSet.stringSets().get(targets).emit(cob); cob.invokestatic(CD_MODULE_BUILDER, "newOpens", MTD_OPENS_MODIFIER_SET_STRING_SET); @@ -241,16 +236,14 @@ Snippet loadOpens(Opens open, int unused) { }; } - private LoadableArray buildOpensArray() { - return LoadableArray.of( - CD_OPENS, - sorted(md.opens()), - this::loadOpens, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Opens", - // number safe for a single page helper under 64K size limit - 2000); + private Snippet buildOpensArray(ClassBuilder clb) { + return new ArraySnippetBuilder(CD_OPENS) + .classBuilder(clb) + .activatePagingThreshold(PAGING_THRESHOLD) + .ownerClassDesc(ownerClassDesc) + .methodNamePrefix("module" + index + "Opens") + .pageSize(2000) // number safe for a single page helper under 64K size limit + .build(Snippet.buildAll(sorted(md.opens()), this::loadOpens)); } /* @@ -260,16 +253,15 @@ private LoadableArray buildOpensArray() { * providers = List.of(new String[] { provide.providers() } * Builder.newProvides(service, providers); */ - private Snippet loadProvides(Provides provide, int offset) { + private Snippet loadProvides(ClassBuilder clb, Provides provide, int offset) { return cob -> { - var providersArray = LoadableArray.of( - CD_String, - provide.providers(), - STRING_LOADER, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Provider" + offset, - STRING_PAGE_SIZE); + var providersArray = new ArraySnippetBuilder(CD_String) + .classBuilder(clb) + .activatePagingThreshold(PAGING_THRESHOLD) + .ownerClassDesc(ownerClassDesc) + .methodNamePrefix("module" + index + "Provider" + offset) + .pageSize(STRING_PAGE_SIZE) + .build(Snippet.buildAll(provide.providers(), Snippet::loadConstant)); cob.loadConstant(provide.service()); providersArray.emit(cob); @@ -283,35 +275,25 @@ private Snippet loadProvides(Provides provide, int offset) { }; } - private LoadableArray buildProvidesArray() { - return LoadableArray.of( - CD_PROVIDES, - sorted(md.provides()), - this::loadProvides, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Provides", - // number safe for a single page helper under 64K size limit - 2000); + private Snippet buildProvidesArray(ClassBuilder clb) { + IndexedElementSnippetBuilder builder = (e, i) -> loadProvides(clb, e, i); + return new ArraySnippetBuilder(CD_PROVIDES) + .classBuilder(clb) + .activatePagingThreshold(PAGING_THRESHOLD) + .ownerClassDesc(ownerClassDesc) + .methodNamePrefix("module" + index + "Provides") + .pageSize(2000) // number safe for a single page helper under 64K size limit + .build(builder.buildAll(md.provides())); } - private LoadableSet buildPackagesSet(Collection packages) { - return LoadableSet.of( - sorted(packages), - STRING_LOADER, - PAGING_THRESHOLD, - ownerClassDesc, - "module" + index + "Packages", - STRING_PAGE_SIZE); - } - - @Override - public void setup(ClassBuilder clb) { - requiresArray.setup(clb); - exportsArray.setup(clb); - opensArray.setup(clb); - providesArray.setup(clb); - packagesSet.setup(clb); + private Snippet buildPackagesSet(ClassBuilder clb, Collection packages) { + return new SetSnippetBuilder(CD_String) + .classBuilder(clb) + .activatePagingThreshold(PAGING_THRESHOLD) + .ownerClassDesc(ownerClassDesc) + .methodNamePrefix("module" + index + "Packages") + .pageSize(STRING_PAGE_SIZE) + .build(Snippet.buildAll(sorted(packages), Snippet::loadConstant)); } @Override @@ -352,7 +334,7 @@ public void emit(CodeBuilder cob) { MTD_OPENS_ARRAY); // uses - dedupSetBuilder.loadStringSet(cob, md.uses()); + dedupSet.stringSets().get(md.uses()).emit(cob); cob.invokevirtual(CD_MODULE_BUILDER, "uses", MTD_SET); 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 483e13814e992..8f124713e115b 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 @@ -57,7 +57,6 @@ import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; -import java.util.function.Consumer; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -87,10 +86,8 @@ import jdk.tools.jlink.plugin.ResourcePoolEntry; import static java.lang.classfile.ClassFile.*; - import static jdk.tools.jlink.internal.Snippets.*; - /** * Jlink plugin to reconstitute module descriptors and other attributes for system * modules. The plugin generates implementations of SystemModules to avoid parsing @@ -528,12 +525,6 @@ byte[] getBytes() throws IOException { static class SystemModulesClassGenerator { private static final ClassDesc CD_MODULE_DESCRIPTOR = ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor"); - private static final ClassDesc CD_REQUIRES_MODIFIER = - ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Requires$Modifier"); - private static final ClassDesc CD_EXPORTS_MODIFIER = - ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Exports$Modifier"); - private static final ClassDesc CD_OPENS_MODIFIER = - ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor$Opens$Modifier"); private static final ClassDesc CD_MODULE_TARGET = ClassDesc.ofInternalName("jdk/internal/module/ModuleTarget"); private static final ClassDesc CD_MODULE_HASHES = @@ -624,7 +615,7 @@ public byte[] genClassBytes(Configuration cf) { genConstructor(clb); // generate dedup set fields and provider methods - genConstants(clb); + var dedupSets = genConstants(clb); // generate hasSplitPackages genHasSplitPackages(clb); @@ -633,7 +624,7 @@ public byte[] genClassBytes(Configuration cf) { genIncubatorModules(clb); // generate moduleDescriptors - genModuleDescriptorsMethod(clb); + genModuleDescriptorsMethod(clb, dedupSets); // generate moduleTargets genModuleTargetsMethod(clb); @@ -664,18 +655,20 @@ private void genConstructor(ClassBuilder clb) { .return_()); } - private void genConstants(ClassBuilder clb) { - var clinitSnippets = dedupSetBuilder.buildConstants(clb); - if (!clinitSnippets.isEmpty()) { + private DedupSet genConstants(ClassBuilder clb) { + var dedupSet = dedupSetBuilder.build(clb); + var clinitSnippet = dedupSet.cacheSetupSnippet(); + if (!clinitSnippet.isEmpty()) { clb.withMethodBody( CLASS_INIT_NAME, MTD_void, ACC_STATIC, cob -> { - clinitSnippets.get().accept(cob); + clinitSnippet.get().emit(cob); cob.return_(); }); } + return dedupSet; } /** @@ -714,19 +707,16 @@ private void genIncubatorModules(ClassBuilder clb) { .ireturn()); } - private void genModuleDescriptorsMethod(ClassBuilder clb) { - var converter = new ModuleDescriptorBuilder(dedupSetBuilder, classDesc); - var moduleDescriptors = LoadableArray.of(CD_MODULE_DESCRIPTOR, - moduleInfos, - converter::build, - moduleDescriptorsPerMethod, - classDesc, - "sub", - moduleDescriptorsPerMethod); - - // This setup helpers needed by the LoadableArray, but element loader is responsible - // to setup elements. - moduleDescriptors.setup(clb); + private void genModuleDescriptorsMethod(ClassBuilder clb, DedupSet dedupSets) { + var converter = new ModuleDescriptorBuilder(clb, dedupSets, classDesc); + var elementSnippets = converter.buildAll(moduleInfos); + var moduleDescriptors = new ArraySnippetBuilder(CD_MODULE_DESCRIPTOR) + .classBuilder(clb) + .activatePagingThreshold(moduleDescriptorsPerMethod) + .ownerClassDesc(classDesc) + .methodNamePrefix("sub") + .pageSize(moduleDescriptorsPerMethod) + .build(elementSnippets); clb.withMethodBody( "moduleDescriptors", @@ -943,8 +933,10 @@ private void generate(ClassBuilder clb, * Generate code to generate an immutable set. */ private void genImmutableSet(CodeBuilder cob, Set set) { - var loadableSet = LoadableSet.of(sorted(set), STRING_LOADER); - loadableSet.emit(cob); + var snippets = Snippet.buildAll(sorted(set), Snippet::loadConstant); + new SetSnippetBuilder(CD_String) + .build(snippets) + .emit(cob); } class ModuleHashesBuilder { @@ -1042,160 +1034,101 @@ void hashForModule(String name, byte[] hash) { /* * Wraps set creation, ensuring identical sets are properly deduplicated. */ - static class DedupSetBuilder { - // map Set to a specialized builder to allow them to be - // deduplicated as they are requested - final Map, SetReference> stringSets = new HashMap<>(); + static record DedupSet(Map, Snippet> stringSets, + Map, Snippet> requiresModifiersSets, + Map, Snippet> opensModifiersSets, + Map, Snippet> exportsModifiersSets, + Optional cacheSetupSnippet) {}; - // map Set to a specialized builder to allow them to be - // deduplicated as they are requested - final Map, SetReference> + static class DedupSetBuilder { + final Map, SetReference> stringSets = new HashMap<>(); + final Map, SetReference> requiresModifiersSets = new HashMap<>(); - - // map Set to a specialized builder to allow them to be - // deduplicated as they are requested - final Map, SetReference> + final Map, SetReference> exportsModifiersSets = new HashMap<>(); - - // map Set to a specialized builder to allow them to be - // deduplicated as they are requested - final Map, SetReference> + final Map, SetReference> opensModifiersSets = new HashMap<>(); - private static final String VALUES_ARRAY = "dedupSetValues"; - final ClassDesc owner; - private final ArrayList values = new ArrayList<>(); + final CacheBuilder cacheBuilder = new CacheBuilder(); + int setBuilt = 0; DedupSetBuilder(ClassDesc owner) { this.owner = owner; } - > SetReference createLoadableSet(Set elements, CollectionElementBuilder elementLoader) { - var loadableSet = LoadableSet.of(sorted(elements), - elementLoader, - PAGING_THRESHOLD, - owner, - "dedupSet" + values.size(), - // Safe for String and Enum within 64K - 3000); - var ref = new SetReference(loadableSet); - values.add(ref); - return ref; - } - /* - * Add the given set of strings to this builder. - */ + * Add the given set of strings to this builder. + */ void stringSet(Set strings) { - stringSets.computeIfAbsent(strings, - s -> createLoadableSet(s, STRING_LOADER) - ).increment(); + stringSets.computeIfAbsent(strings, SetReference::new).increment(); } /* - * Add the given set of Exports.Modifiers - */ + * Add the given set of Exports.Modifiers + */ void exportsModifiers(Set mods) { - exportsModifiersSets.computeIfAbsent(mods, - s -> createLoadableSet(s, (export, _) -> new EnumConstant(export)) - ).increment(); + exportsModifiersSets.computeIfAbsent(mods, SetReference::new).increment(); } /* - * Add the given set of Opens.Modifiers - */ + * Add the given set of Opens.Modifiers + */ void opensModifiers(Set mods) { - opensModifiersSets.computeIfAbsent(mods, - s -> createLoadableSet(s, (open, _) -> new EnumConstant(open)) - ).increment(); + opensModifiersSets.computeIfAbsent(mods, SetReference::new).increment(); } /* - * Add the given set of Requires.Modifiers - */ + * Add the given set of Requires.Modifiers + */ void requiresModifiers(Set mods) { - requiresModifiersSets.computeIfAbsent(mods, - s -> createLoadableSet(s, (require, _) -> new EnumConstant(require)) - ).increment(); + requiresModifiersSets.computeIfAbsent(mods, SetReference::new).increment(); } - /* - * Load the given set to the top of operand stack. - */ - void loadStringSet(CodeBuilder cob, Set names) { - stringSets.get(names).emit(cob); + private Snippet buildStringSet(ClassBuilder clb, SetReference setRef) { + return cacheBuilder.transform(setRef, + new SetSnippetBuilder(CD_String) + .classBuilder(clb) + .activatePagingThreshold(PAGING_THRESHOLD) + .ownerClassDesc(owner) + .methodNamePrefix("dedupSet" + setBuilt++) + .pageSize(STRING_PAGE_SIZE) + .build(Snippet.buildAll(setRef.elements(), Snippet::loadConstant))); } - /* - * Load the given set to the top of operand stack. - */ - void loadExportsModifiers(CodeBuilder cob, Set mods) { - exportsModifiersSets.get(mods).emit(cob); + private > Snippet buildEnumSet(ClassBuilder clb, SetReference setRef) { + return cacheBuilder.transform(setRef, + new SetSnippetBuilder(CD_Object) + .classBuilder(clb) + .activatePagingThreshold(PAGING_THRESHOLD) + .ownerClassDesc(owner) + .methodNamePrefix("dedupSet" + setBuilt++) + .pageSize(ENUM_PAGE_SIZE) + .build(Snippet.buildAll(setRef.elements(), Snippet::loadEnum))); } - /* - * Load the given set to the top of operand stack. - */ - void loadOpensModifiers(CodeBuilder cob, Set mods) { - opensModifiersSets.get(mods).emit(cob); + private Map, Snippet> buildStringSets(ClassBuilder clb, Map, SetReference> map) { + Map, Snippet> snippets = new HashMap<>(map.size()); + map.entrySet().forEach(entry -> snippets.put(entry.getKey(), + buildStringSet(clb, entry.getValue()))); + return snippets; } - - /* - * Load the given set to the top of operand stack. - */ - void loadRequiresModifiers(CodeBuilder cob, Set mods) { - requiresModifiersSets.get(mods).emit(cob); + private > Map, Snippet> buildEnumSets(ClassBuilder clb, Map, SetReference> map) { + Map, Snippet> snippets = new HashMap<>(map.size()); + map.entrySet().forEach(entry -> snippets.put(entry.getKey(), + buildEnumSet(clb, entry.getValue()))); + return snippets; } - /* - * Adding provider methods to the class. For those set used more than once, built - * once and keep the reference for later access. - * Return a snippet to setup the cache . - * - * The returned snippet would set up the set referenced more than once, - * - * static final Set[] dedupSetValues; - * - * static { - * dedupSetValues = new Set[countOfStoredValues]; - * dedupSetValues[0] = Set.of(elements); // elements no more than SET_SIZE_THRESHOLD - * dedupSetValues[1] = dedupProvider(); // set elements more than SET_SIZE_THRESHOLD - * ... - * dedupSetValues[countOfStoredValues - 1] = ... - * } - */ - Optional> buildConstants(ClassBuilder clb) { - var staticCache = new ArrayList(); - - for (var set: values) { - set.setup(clb); - if (set.refCount() > 1) { - staticCache.add(set); - } - } - - if (staticCache.isEmpty()) { - return Optional.empty(); - } - - var cacheValuesArray = LoadableArray.of( - CD_Set, - staticCache, - SetReference::build, - PAGING_THRESHOLD, - owner, - VALUES_ARRAY, - 2000); - - cacheValuesArray.setup(clb); - clb.withField(VALUES_ARRAY, CD_Set.arrayType(), ACC_STATIC | ACC_FINAL); - - return Optional.of(cob -> { - cacheValuesArray.emit(cob); - cob.putstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); - }); + DedupSet build(ClassBuilder clb) { + return new DedupSet( + buildStringSets(clb, stringSets), + buildEnumSets(clb, requiresModifiersSets), + buildEnumSets(clb, opensModifiersSets), + buildEnumSets(clb, exportsModifiersSets), + cacheBuilder.build(clb) + ); } /* @@ -1205,16 +1138,13 @@ Optional> buildConstants(ClassBuilder clb) { * and load from there. Otherwise, the set is built in place and load onto the operand * stack. */ - class SetReference implements Snippet { + class SetReference> { // sorted elements of the set to ensure same generated code - private final LoadableSet loadableSet; - + private final List elements; private int refCount; - // The index for this set value in the cache array - private int index = -1; - SetReference(LoadableSet set) { - this.loadableSet = set; + SetReference(Set elements) { + this.elements = sorted(elements); } int increment() { @@ -1225,37 +1155,73 @@ int refCount() { return refCount; } + List elements() { + return elements; + } + } + + class CacheBuilder { + private static final String VALUES_ARRAY = "dedupSetValues"; + final ArrayList cachedValues = new ArrayList<>(); + // Load the set to the operand stack. // When referenced more than once, the value is pre-built with static initialzer // and is load from the cache array with // dedupSetValues[index] // Otherwise, LoadableSet will load the set onto the operand stack. - @Override - public void emit(CodeBuilder cob) { - if (refCount > 1) { - assert index >= 0; - cob.getstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); - cob.loadConstant(index); - cob.aaload(); + private Snippet loadFromCache(int index) { + assert index >= 0; + return cob -> + cob.getstatic(owner, VALUES_ARRAY, CD_Set.arrayType()) + .loadConstant(index) + .aaload(); + } + + Snippet transform(SetReference setRef, Snippet loadSnippet) { + if (setRef.refCount() > 1) { + cachedValues.add(loadSnippet); + return loadFromCache(cachedValues.size() - 1); } else { - loadableSet.emit(cob); + return loadSnippet; } } - @Override - public void setup(ClassBuilder clb) { - loadableSet.setup(clb); - } + /* + * Adding provider methods to the class. For those set used more than once, built + * once and keep the reference for later access. + * Return a snippet to setup the cache . + * + * The returned snippet would set up the set referenced more than once, + * + * static final Set[] dedupSetValues; + * + * static { + * dedupSetValues = new Set[countOfStoredValues]; + * dedupSetValues[0] = Set.of(elements); // elements no more than SET_SIZE_THRESHOLD + * dedupSetValues[1] = dedupProvider(); // set elements more than SET_SIZE_THRESHOLD + * ... + * dedupSetValues[countOfStoredValues - 1] = ... + * } + */ + Optional build(ClassBuilder clb) { + if (cachedValues.isEmpty()) { + return Optional.empty(); + } + + var cacheValuesArray = new ArraySnippetBuilder(CD_Set) + .classBuilder(clb) + .activatePagingThreshold(PAGING_THRESHOLD) + .ownerClassDesc(owner) + .methodNamePrefix(VALUES_ARRAY) + .pageSize(2000) + .build(cachedValues.toArray(Snippet[]::new)); - /** - * Build the snippet to load the set onto the operand stack for storing into cache - * to be later accessed by the SetReference::emit - */ - Snippet build(int index) { - this.index = index; - return cob -> { - loadableSet.emit(cob); - }; + clb.withField(VALUES_ARRAY, CD_Set.arrayType(), ACC_STATIC | ACC_FINAL); + + return Optional.of(cob -> { + cacheValuesArray.emit(cob); + cob.putstatic(owner, VALUES_ARRAY, CD_Set.arrayType()); + }); } } } diff --git a/test/jdk/tools/jlink/SnippetsTest.java b/test/jdk/tools/jlink/SnippetsTest.java index 5bc83a7b0f4cd..b2f3443bc138f 100644 --- a/test/jdk/tools/jlink/SnippetsTest.java +++ b/test/jdk/tools/jlink/SnippetsTest.java @@ -24,6 +24,7 @@ */ import java.io.IOException; +import java.lang.classfile.ClassBuilder; import java.lang.classfile.ClassFile; import static java.lang.classfile.ClassFile.ACC_PUBLIC; import java.lang.constant.ClassDesc; @@ -40,6 +41,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.*; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.IntStream; @@ -49,7 +51,6 @@ import static org.junit.jupiter.api.Assertions.*; import jdk.tools.jlink.internal.Snippets.*; -import static jdk.tools.jlink.internal.Snippets.*; /* * @test @@ -91,40 +92,6 @@ void testStringArrayLimitsWithoutPagination() { } } - @ParameterizedTest - @ValueSource(booleans = { true, false }) - void testLoadableProvider(boolean isStatic) throws NoSuchMethodException { - var expected = IntStream.range(0, 1234) - .mapToObj(i -> "WrapperTestString" + i) - .toList(); - var className = "WrapperLoadableTest" + (isStatic ? "Static" : "Public"); - ClassDesc testClassDesc = ClassDesc.of(className); - - var loadable = new PaginatedArray<>( - CD_String, expected, STRING_LOADER, testClassDesc, "page", 100); - // 1234 with 10 per page, should have 13 pages with last page 34 elements - assertEquals(13, loadable.pageCount()); - assertTrue(loadable.isLastPagePartial()); - - var provider = new LoadableProvider(loadable, testClassDesc, "wrapper", isStatic); - Supplier supplier = generateSupplier(className, provider, loadable); - verifyPaginationMethods(supplier.getClass(), String.class, "page", 13); - assertArrayEquals(expected.toArray(), supplier.get()); - - // check wrapper function - var methodType = MethodType.methodType(String[].class); - try { - lookup().findStatic(supplier.getClass(), provider.methodName(), methodType); - } catch (IllegalAccessException ex) { - assertFalse(isStatic); - } - try { - lookup().findVirtual(supplier.getClass(), provider.methodName(), methodType); - } catch (IllegalAccessException ex) { - assertTrue(isStatic); - } - } - @Test void testLoadableEnum() { Enum[] enums = { @@ -134,78 +101,82 @@ void testLoadableEnum() { ModuleDescriptor.Requires.Modifier.TRANSITIVE }; - var loadable = new SimpleArray( - Enum.class.describeConstable().get(), - Arrays.stream(enums).map(EnumConstant::new).toList(), - (enumConstant, _) -> enumConstant); + Snippet[] elementSnippets = Snippet.buildAll(Arrays.asList(enums), Snippet::loadEnum); + + var loadable = new ArraySnippetBuilder(Enum.class.describeConstable().get()) + .build(elementSnippets); - Supplier[]> supplier = generateSupplier("LoadableEnumTest", loadable); + Supplier[]> supplier = generateSupplier("LoadableEnumTest", clb -> loadable); assertArrayEquals(enums, supplier.get()); } @Test - void testLoadableArrayOf() { + void testArraySnippetBuilder() { Integer[] expected = IntStream.range(0, 200) .boxed() .toArray(Integer[]::new); var className = "LoadableArrayOf200Paged"; - var loadable = LoadableArray.of(CD_Integer, - Arrays.asList(expected), - INTEGER_LOADER, - expected.length - 1, - ClassDesc.of(className), - "page", - 100); - assertTrue(loadable instanceof PaginatedArray); - - Supplier supplier = generateSupplier(className, loadable); + var elementSnippets = Snippet.buildAll(Arrays.asList(expected), Snippet::loadInteger); + var instance = new ArraySnippetBuilder(CD_Integer) + .activatePagingThreshold(expected.length - 1) + .ownerClassDesc(ClassDesc.of(className)) + .methodNamePrefix("page") + .pageSize(100); + + Supplier supplier = generateSupplier(className, clb -> instance.classBuilder(clb).build(elementSnippets)); verifyPaginationMethods(supplier.getClass(), Integer.class, "page", 2); assertArrayEquals(expected, supplier.get()); - loadable = LoadableArray.of( - CD_Integer, - Arrays.asList(expected), - INTEGER_LOADER, - expected.length, - ClassDesc.of("LoadableArrayOf200NotPaged"), - "page", - 100); - assertTrue(loadable instanceof SimpleArray); + var loadable = instance.activatePagingThreshold(expected.length) + .ownerClassDesc(ClassDesc.of("LoadableArrayOf200NotPaged")) + .methodNamePrefix("page") + .pageSize(100) + .build(elementSnippets); // SimpleArray generate bytecode inline, so can be generated in any class - supplier = generateSupplier("TestLoadableArrayFactory", loadable); + supplier = generateSupplier("TestLoadableArrayFactory", clb -> loadable); + verifyPaginationMethods(supplier.getClass(), Integer.class, "page", 0); assertArrayEquals(expected, supplier.get()); } @Test - void testLoadableSetOf() { + void testSetSnippetBuilder() { String[] data = IntStream.range(0, 100) .mapToObj(i -> "SetData" + i) .toArray(String[]::new); var tiny = Set.of(data[0], data[1], data[2]); var all = Set.of(data); + var setBuilder = new SetSnippetBuilder(CD_String); - Supplier> supplier = generateSupplier("TinySetTest", LoadableSet.of(tiny, STRING_LOADER)); + Supplier> supplier = generateSupplier("TinySetTest", clb -> + setBuilder.build(Snippet.buildAll(tiny, Snippet::loadConstant))); // Set does not guarantee ordering, so not assertIterableEquals assertEquals(tiny, supplier.get()); - supplier = generateSupplier("AllSetTestNoPage", LoadableSet.of(all, STRING_LOADER)); + var allSnippets = Snippet.buildAll(all, Snippet::loadConstant); + + supplier = generateSupplier("AllSetTestNoPage", clb -> + setBuilder.build(allSnippets)); assertEquals(all, supplier.get()); var className = "AllSetTestPageNotActivated"; var methodNamePrefix = "page"; - var loadable = LoadableSet.of(all, STRING_LOADER, all.size(), - ClassDesc.of(className), methodNamePrefix, 10); - supplier = generateSupplier(className, loadable); + var loadable = setBuilder.activatePagingThreshold(all.size()) + .ownerClassDesc(ClassDesc.of(className)) + .methodNamePrefix(methodNamePrefix) + .pageSize(10) + .build(allSnippets); + supplier = generateSupplier(className, clb -> loadable); assertEquals(all, supplier.get()); className = "AllSetTestPageSize20"; - loadable = LoadableSet.of(all, STRING_LOADER, all.size() - 1, - ClassDesc.of(className), methodNamePrefix, 20); - supplier = generateSupplier(className, loadable); - // Set erased element type and use Object as element type - verifyPaginationMethods(supplier.getClass(), Object.class, methodNamePrefix, 5); + setBuilder.ownerClassDesc(ClassDesc.of(className)); + supplier = generateSupplier(className, clb -> setBuilder.classBuilder(clb) + .activatePagingThreshold(all.size() - 1) + .pageSize(20) + .build(allSnippets)); + verifyPaginationMethods(supplier.getClass(), String.class, methodNamePrefix, 5); assertEquals(all, supplier.get()); } @@ -215,12 +186,17 @@ void testPaginatedArray(int elementCount, int pageSize) { .toArray(String[]::new); var className = String.format("SnippetArrayProviderTest%dPagedBy%d", elementCount, pageSize); ClassDesc testClassDesc = ClassDesc.of(className); - var loadable = new PaginatedArray<>(CD_String, expected, STRING_LOADER, - testClassDesc, "ArrayPage", pageSize); - - Supplier supplier = generateSupplier(className, loadable); - verifyPaginationMethods(supplier.getClass(), String.class, "ArrayPage", loadable.pageCount()); - assertEquals((elementCount % pageSize) != 0, loadable.isLastPagePartial()); + var builder = new ArraySnippetBuilder(CD_String) + .activatePagingThreshold(1) + .ownerClassDesc(testClassDesc) + .methodNamePrefix("ArrayPage") + .pageSize(pageSize); + var snippets = Snippet.buildAll(Arrays.asList(expected), Snippet::loadConstant); + var pagingContext = new PagingContext(expected.length, pageSize); + + Supplier supplier = generateSupplier(className, clb -> builder.classBuilder(clb).build(snippets)); + verifyPaginationMethods(supplier.getClass(), String.class, "ArrayPage", pagingContext.pageCount()); + assertEquals((elementCount % pageSize) != 0, pagingContext.isLastPagePartial()); assertArrayEquals(expected, supplier.get()); } @@ -229,15 +205,16 @@ void testSimpleArray(int elementCount) { .mapToObj(i -> "NoPage" + i) .toArray(String[]::new); String className = "SnippetArrayProviderTest" + elementCount; - var array = new SimpleArray<>(CD_String, Arrays.asList(expected), STRING_LOADER); + var array = new ArraySnippetBuilder(CD_String) + .build(Snippet.buildAll(Arrays.asList(expected), Snippet::loadConstant)); - Supplier supplier = generateSupplier(className, array); + Supplier supplier = generateSupplier(className, clb -> array); assertArrayEquals(expected, supplier.get()); } - Supplier generateSupplier(String className, Loadable loadable, Loadable... extra) { + Supplier generateSupplier(String className, Function builder) { var testClassDesc = ClassDesc.of(className); - byte[] classBytes = generateSupplierClass(testClassDesc, loadable, extra); + byte[] classBytes = generateSupplierClass(testClassDesc, builder); try { writeClassFile(className, classBytes); var testClass = lookup().defineClass(classBytes); @@ -249,17 +226,24 @@ Supplier generateSupplier(String className, Loadable loadable, Loadable.. } void verifyPaginationMethods(Class testClass, Class elementType, String methodNamePrefix, int pageCount) { + var methodType = MethodType.methodType(elementType.arrayType(), elementType.arrayType()); + if (pageCount <= 0) { + try { + lookup().findStatic(testClass, methodNamePrefix + 0, methodType); + fail("Unexpected paginate helper function"); + } catch (Exception ex) {} + } + for (int i = 0; i < pageCount; i++) { try { - lookup().findStatic(testClass, methodNamePrefix + i, - MethodType.methodType(elementType.arrayType(), elementType.arrayType())); + lookup().findStatic(testClass, methodNamePrefix + i, methodType); } catch (Exception ex) { throw new RuntimeException(ex); } } } - byte[] generateSupplierClass(ClassDesc testClassDesc, Loadable loadable, Loadable... extra) { + byte[] generateSupplierClass(ClassDesc testClassDesc, Function builder) { return ClassFile.of().build(testClassDesc, clb -> { clb.withSuperclass(CD_Object); @@ -270,13 +254,7 @@ byte[] generateSupplierClass(ClassDesc testClassDesc, Loadable loadable, Loadabl cob.return_(); }); - loadable.setup(clb); - - for (var e: extra) { - // always call setup should be no harm - // it suppose to be nop if not required. - e.setup(clb); - } + var loadable = builder.apply(clb); clb.withMethodBody("get", MethodTypeDesc.of(CD_Object), ACC_PUBLIC, cob -> { loadable.emit(cob); From 4833be888c51dd9beb039be51ab5a52d1aeab5c1 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Thu, 19 Dec 2024 00:29:31 -0800 Subject: [PATCH 13/15] Javadoc and some minor refactoring --- .../jdk/tools/jlink/internal/Snippets.java | 194 +++++++++++++++--- .../plugins/ModuleDescriptorBuilder.java | 27 +-- .../internal/plugins/SystemModulesPlugin.java | 189 ++++++++++------- test/jdk/tools/jlink/SnippetsTest.java | 30 +-- 4 files changed, 308 insertions(+), 132 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java index a22e2d93816ce..430f4007a258f 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -34,6 +34,7 @@ import java.util.Objects; import static java.lang.classfile.ClassFile.ACC_STATIC; +import java.lang.classfile.TypeKind; import java.lang.constant.ConstantDesc; import static java.lang.constant.ConstantDescs.CD_Integer; import static java.lang.constant.ConstantDescs.CD_Object; @@ -42,10 +43,6 @@ import java.util.function.Function; public class Snippets { - // Tested page size of string array - public static final int STRING_PAGE_SIZE = 8000; - // Tested page size of enum array - public static final int ENUM_PAGE_SIZE = 5000; /** * Snippet of bytecodes */ @@ -54,26 +51,40 @@ public interface Snippet { /** * Emit the bytecode snippet to the CodeBuilder. * - * @param cob The CodeBuilder the bytecode snippet. - * @throws IllegalStateException If the snippet is not setup properly. + * @param cob The CodeBuilder */ void emit(CodeBuilder cob); + /** + * Load a constant onto the operand stack. + */ static Snippet loadConstant(T v) { return cob -> cob.loadConstant(v); } + /** + * Load an enum constant onto the operand stack. + */ static Snippet loadEnum(Enum e) { var classDesc = e.getClass().describeConstable().get(); return cob -> cob.getstatic(classDesc, e.name(), classDesc); } + /** + * Load an Integer, boxed int value onto the operand stack. + */ static Snippet loadInteger(int value) { return cob -> cob.loadConstant(value) - .invokestatic(ClassDesc.ofInternalName("java/lang/Integer"), "valueOf", MethodTypeDesc.of(CD_Integer, CD_int)); + .invokestatic(CD_Integer, "valueOf", MethodTypeDesc.of(CD_Integer, CD_int)); } + /** + * Build snippets each to process the corresponding element. + * @param elements The elements to be processed + * @param fn The snippet building function for a given element + * @return Snippets + */ static Snippet[] buildAll(Collection elements, Function fn) { return elements.stream() .map(fn) @@ -82,8 +93,11 @@ static Snippet[] buildAll(Collection elements, Function fn) { } /** - * Describe a reference that can be load onto the operand stack. + * Describe a operand that can be load onto the operand stack. * For example, an array of string can be described as a Loadable. + * + * @param classDesc The type of the operand + * @param load The snippet to load the operand onto the operand stack */ public record Loadable(ClassDesc classDesc, Snippet load) implements Snippet { /** @@ -96,6 +110,10 @@ public void emit(CodeBuilder cob) { } } + /** + * Build a snippet for an element with a given index. Typically used for elements in a + * collection to identify the specific element. + */ @FunctionalInterface public interface IndexedElementSnippetBuilder { /** @@ -117,16 +135,30 @@ default Snippet[] buildAll(Collection elements) { } } + /** + * Some basic information about pagination. + * @param total The total count of elements + * @param pageSize the number or elements to be included in a page + */ public static record PagingContext(int total, int pageSize) { + /** + * If the last page has less elements than given page size. + */ public boolean isLastPagePartial() { return (total % pageSize) != 0; } + /** + * The number of pages. + */ public int pageCount() { var pages = total / pageSize; return isLastPagePartial() ? pages + 1 : pages; } + /** + * The number of elements in the last page. + */ public int lastPageSize() { if (total == 0) return 0; var remaining = total % pageSize; @@ -134,70 +166,170 @@ public int lastPageSize() { } } + /** + * Generate bytecodes for loading a collection of elements, support using pagination to avoid + * overloading the 64k code limit. + */ public static abstract class CollectionSnippetBuilder { + /** + * Tested page size of string array + */ + public static final int STRING_PAGE_SIZE = 8000; + + /** + * Tested page size of enum array + */ + public static final int ENUM_PAGE_SIZE = 5000; + + /** + * Good enough for average ~30 bytes per element + */ + public static final int DEFAULT_PAGE_SIZE = 2000; + + /** + * Arbitary default values based on 15K code size on ~30 bytes per element + */ + protected static final int DEFAULT_THRESHOLD = 512; + protected ClassDesc elementType; - protected int activatePagingThreshold; protected ClassDesc ownerClassDesc; - protected String methodNamePrefix; - protected int pageSize; protected ClassBuilder clb; + // Default values enable pagination by default + protected String methodNamePrefix = "csb" + Integer.toHexString(hashCode()) + "Page"; + protected int activatePagingThreshold = DEFAULT_THRESHOLD; + protected int pageSize = DEFAULT_PAGE_SIZE; + + /** + * @param elementType The element type + */ protected CollectionSnippetBuilder(ClassDesc elementType) { this.elementType = Objects.requireNonNull(elementType); } /** + * Enable pagination if the count of elements is larger than the given threshold. + * + * @param methodNamePrefix The method name prefix for generated paging helper methods + * @param pageSize The page size + * @param threshold The element count to actiave the pagination + */ + public CollectionSnippetBuilder enablePagination(String methodNamePrefix, int pageSize, int threshold) { + return this.pageSize(pageSize) + .activatePagingThreshold(threshold) + .methodNamePrefix(methodNamePrefix); + } + + /** + * Enable pagination if the count of elements is larger than pageSize or DEFAULT_THRESHOLD + */ + public CollectionSnippetBuilder enablePagination(String methodNamePrefix, int pageSize) { + return enablePagination(methodNamePrefix, pageSize, Math.min(pageSize, DEFAULT_THRESHOLD)); + } + + /** + * Enable pagination if the count of elements is larger than pageSize or DEFAULT_THRESHOLD + * with page size DEFAULT_PAGE_SIZE. + */ + public CollectionSnippetBuilder enablePagination(String methodNamePrefix) { + return enablePagination(methodNamePrefix, DEFAULT_PAGE_SIZE, DEFAULT_THRESHOLD); + } + + /** + * Disable pagination. Generated bytecode will always try to construct the collection inline. + */ + public CollectionSnippetBuilder disablePagination() { + this.activatePagingThreshold = -1; + this.methodNamePrefix = null; + return this; + } + + /** + * Set the threshold of element count to enable pagination. + * * @param activatePagingThreshold Use pagination methods if the count of elements is larger * than the given value */ public CollectionSnippetBuilder activatePagingThreshold(int activatePagingThreshold) { + if (activatePagingThreshold <= 0) { + throw new IllegalArgumentException(); + } this.activatePagingThreshold = activatePagingThreshold; return this; } /** - * @param ownerClassDesc The owner class for the paginattion methods + * Set the owner class host the pagination methods. + * + * @param ownerClassDesc The owner class for the pagination methods */ public CollectionSnippetBuilder ownerClassDesc(ClassDesc ownerClassDesc) { - this.ownerClassDesc = ownerClassDesc; + this.ownerClassDesc = Objects.requireNonNull(ownerClassDesc); return this; } /** + * Set the method name prefix for the pagination methods. * @param methodNamePrefix The method name prefix. Generated method will have the name of * this value appended with page number */ public CollectionSnippetBuilder methodNamePrefix(String methodNamePrefix) { - this.methodNamePrefix = methodNamePrefix; + this.methodNamePrefix = Objects.requireNonNull(methodNamePrefix); return this; } /** + * Set the page size. The max page size is STRING_PAGE_SIZE. * @param pageSize The count of elements per page* */ public CollectionSnippetBuilder pageSize(int pageSize) { + // ldc is likely the smallest element snippet + if (pageSize <= 0 || pageSize > STRING_PAGE_SIZE) { + throw new IllegalArgumentException(); + } + this.pageSize = pageSize; return this; } + /** + * Set the class builder used to generate the pagination methods. + * + * This value must be set if pagination is needed, otherwise the build + * would lead to NullPointerException. + */ public CollectionSnippetBuilder classBuilder(ClassBuilder clb) { - this.clb = clb; + this.clb = Objects.requireNonNull(clb); return this; } protected boolean shouldPaginate(int length) { - return activatePagingThreshold != 0 && length > activatePagingThreshold; + return activatePagingThreshold > 0 && length > activatePagingThreshold; } + /** + * Build the Loadable snippet to load the collection of elements onto + * the operand stack. When pagination is enabled and needed as the total + * count of elements is larger than the given threshold, missing + * required field will lead to NullPointerException. + * + * @param loadElementSnippets The array of Snippets used to load individual + * element in the collection. + * @return The Loadable snippet + * @throws NullPointerException + */ abstract public Loadable build(Snippet[] loadElementSnippets); } /** - * Generate bytecode for pagination methods, then create the array inline and invoke the first page method to assign - * values to the array. Each pagination method will assign value to the corresponding page and chain calling next - * page. - * {@code setup} must be called to generate the pagination methods in the owner class. Otherwise, {@code load} will - * lead to {@link java.lang.NoSuchMethodException} + * Generate bytecode to load an array of the given referene type onto the operand stack. + * + * The generated code will create an array inline, and then populate the array either inline or + * by invoking the first pagination method if pagination is activated. + * + * If pagination is activated, pagination methods are generated with the given ClassBuilder + * with method name formatted with the methodNamePrefix appended with page numberand. + * Each pagination method will assign value to the corresponding page and chain calling next page. * * Effectively as * methodNamePrefix0(new T[elements.size()]); @@ -251,6 +383,8 @@ private void newArray(CodeBuilder cob) { */ private void setupHelpers() { Objects.requireNonNull(clb); + Objects.requireNonNull(methodNamePrefix); + Objects.requireNonNull(ownerClassDesc); var lastPageNo = new PagingContext(loadElementSnippets.length, pageSize).pageCount() - 1; for (int pageNo = 0; pageNo <= lastPageNo; pageNo++) { genFillPageHelper(pageNo, pageNo < lastPageNo); @@ -265,20 +399,19 @@ private void genFillPageHelper(int pageNo, boolean hasNextPage) { clb.withMethodBody(methodNamePrefix + pageNo, MTD_PageHelper, ACC_STATIC, - mcob -> { - mcob.aload(0); // arrayref - fill(mcob, fromIndex, toIndex); + cob -> { + cob.aload(0); // arrayref + fill(cob, fromIndex, toIndex); if (hasNextPage) { - mcob.invokestatic( + cob.invokestatic( ownerClassDesc, methodNamePrefix + (pageNo + 1), MTD_PageHelper); } - mcob.areturn(); + cob.return_(TypeKind.from(classDesc)); }); } - @Override public Loadable build(Snippet[] loadElementSnippets) { this.loadElementSnippets = Objects.requireNonNull(loadElementSnippets); @@ -291,7 +424,12 @@ public Loadable build(Snippet[] loadElementSnippets) { } } - // Set support + /** + * Generate bytecodes to load a set onto the operand stack. + * + * The Set is constructed with Set::of method. When there are more than 2 + * elements in the set, an array is constructed. + */ public static class SetSnippetBuilder extends ArraySnippetBuilder { public SetSnippetBuilder(ClassDesc elementType) { super(elementType); diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java index 6efa4f49be509..a0d9cda16252a 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java @@ -42,17 +42,20 @@ import java.util.Set; import static jdk.tools.jlink.internal.Snippets.*; +import static jdk.tools.jlink.internal.Snippets.CollectionSnippetBuilder.STRING_PAGE_SIZE; import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.ModuleInfo; import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.SystemModulesClassGenerator.DedupSet; +/** + * Build a Snippet to load a ModuleDescriptor onto the operand stack. + */ class ModuleDescriptorBuilder implements IndexedElementSnippetBuilder { private static final ClassDesc CD_MODULE_DESCRIPTOR = ClassDesc.ofInternalName("java/lang/module/ModuleDescriptor"); private static final ClassDesc CD_MODULE_BUILDER = ClassDesc.ofInternalName("jdk/internal/module/Builder"); - private static final int PAGING_THRESHOLD = 512; private final DedupSet dedupSet; private final ClassDesc ownerClassDesc; private final ClassBuilder clb; @@ -159,11 +162,9 @@ Snippet loadRequire(Requires require) { private Snippet buildRequiresArray(ClassBuilder clb) { return new ArraySnippetBuilder(CD_REQUIRES) + .enablePagination("module" + index + "Requires") .classBuilder(clb) - .activatePagingThreshold(PAGING_THRESHOLD) .ownerClassDesc(ownerClassDesc) - .methodNamePrefix("module" + index + "Requires") - .pageSize(2000) // number safe for a single page helper under 64K size limit .build(Snippet.buildAll(sorted(md.requires()), this::loadRequire)); } @@ -199,10 +200,8 @@ Snippet loadExports(Exports export) { private Snippet buildExportsArray(ClassBuilder clb) { return new ArraySnippetBuilder(CD_EXPORTS) .classBuilder(clb) - .activatePagingThreshold(PAGING_THRESHOLD) .ownerClassDesc(ownerClassDesc) - .methodNamePrefix("module" + index + "Exports") - .pageSize(2000) // number safe for a single page helper under 64K size limit + .enablePagination("module" + index + "Exports") .build(Snippet.buildAll(sorted(md.exports()), this::loadExports)); } @@ -239,10 +238,8 @@ Snippet loadOpens(Opens open) { private Snippet buildOpensArray(ClassBuilder clb) { return new ArraySnippetBuilder(CD_OPENS) .classBuilder(clb) - .activatePagingThreshold(PAGING_THRESHOLD) .ownerClassDesc(ownerClassDesc) - .methodNamePrefix("module" + index + "Opens") - .pageSize(2000) // number safe for a single page helper under 64K size limit + .enablePagination("module" + index + "Opens") .build(Snippet.buildAll(sorted(md.opens()), this::loadOpens)); } @@ -257,9 +254,8 @@ private Snippet loadProvides(ClassBuilder clb, Provides provide, int offset) { return cob -> { var providersArray = new ArraySnippetBuilder(CD_String) .classBuilder(clb) - .activatePagingThreshold(PAGING_THRESHOLD) .ownerClassDesc(ownerClassDesc) - .methodNamePrefix("module" + index + "Provider" + offset) + .enablePagination("module" + index + "Provider" + offset) .pageSize(STRING_PAGE_SIZE) .build(Snippet.buildAll(provide.providers(), Snippet::loadConstant)); @@ -279,19 +275,16 @@ private Snippet buildProvidesArray(ClassBuilder clb) { IndexedElementSnippetBuilder builder = (e, i) -> loadProvides(clb, e, i); return new ArraySnippetBuilder(CD_PROVIDES) .classBuilder(clb) - .activatePagingThreshold(PAGING_THRESHOLD) .ownerClassDesc(ownerClassDesc) - .methodNamePrefix("module" + index + "Provides") - .pageSize(2000) // number safe for a single page helper under 64K size limit + .enablePagination("module" + index + "Provides") .build(builder.buildAll(md.provides())); } private Snippet buildPackagesSet(ClassBuilder clb, Collection packages) { return new SetSnippetBuilder(CD_String) .classBuilder(clb) - .activatePagingThreshold(PAGING_THRESHOLD) .ownerClassDesc(ownerClassDesc) - .methodNamePrefix("module" + index + "Packages") + .enablePagination("module" + index + "Packages") .pageSize(STRING_PAGE_SIZE) .build(Snippet.buildAll(sorted(packages), Snippet::loadConstant)); } 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 8f124713e115b..0492a78fe396f 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 @@ -87,6 +87,8 @@ import static java.lang.classfile.ClassFile.*; import static jdk.tools.jlink.internal.Snippets.*; +import static jdk.tools.jlink.internal.Snippets.CollectionSnippetBuilder.ENUM_PAGE_SIZE; +import static jdk.tools.jlink.internal.Snippets.CollectionSnippetBuilder.STRING_PAGE_SIZE; /** * Jlink plugin to reconstitute module descriptors and other attributes for system @@ -331,7 +333,7 @@ private String genSystemModulesClass(List moduleInfos, = new SystemModulesClassGenerator(className, moduleInfos, moduleDescriptorsPerMethod); byte[] bytes = generator.genClassBytes(cf); // Diagnosis help, can be removed - if (Boolean.parseBoolean(System.getProperty("JlinkDumpSystemModuleClass", "false"))) { + if (Boolean.parseBoolean(System.getProperty("jlink.dumpSystemModuleClass", "false"))) { try { var filePath = Path.of(className + ".class").toAbsolutePath(); System.err.println("Write " + filePath.toString()); @@ -543,8 +545,6 @@ static class SystemModulesClassGenerator { private static final MethodTypeDesc MTD_MapEntry_Object_Object = MethodTypeDesc.of(CD_Map_Entry, CD_Object, CD_Object); private static final MethodTypeDesc MTD_Map_MapEntryArray = MethodTypeDesc.of(CD_Map, CD_Map_Entry.arrayType()); - static final int PAGING_THRESHOLD = 512; // An arbitrary number as this likely generate minimum ~4K code - private static final int MAX_LOCAL_VARS = 256; private final int MT_VAR = 1; // variable for ModuleTarget @@ -563,6 +563,7 @@ static class SystemModulesClassGenerator { // names or modifiers to reduce the footprint // e.g. target modules of qualified exports private final DedupSetBuilder dedupSetBuilder; + private final ArrayList clinitSnippets = new ArrayList<>(); public SystemModulesClassGenerator(String className, List moduleInfos, @@ -581,19 +582,19 @@ public SystemModulesClassGenerator(String className, */ private void dedups(ModuleDescriptor md) { // exports - for (Exports e : sorted(md.exports())) { + for (Exports e : md.exports()) { dedupSetBuilder.stringSet(e.targets()); dedupSetBuilder.exportsModifiers(e.modifiers()); } // opens - for (Opens opens : sorted(md.opens())) { + for (Opens opens : md.opens()) { dedupSetBuilder.stringSet(opens.targets()); dedupSetBuilder.opensModifiers(opens.modifiers()); } // requires - for (Requires r : sorted(md.requires())) { + for (Requires r : md.requires()) { dedupSetBuilder.requiresModifiers(r.modifiers()); } @@ -614,9 +615,6 @@ public byte[] genClassBytes(Configuration cf) { // generate genConstructor(clb); - // generate dedup set fields and provider methods - var dedupSets = genConstants(clb); - // generate hasSplitPackages genHasSplitPackages(clb); @@ -624,7 +622,7 @@ public byte[] genClassBytes(Configuration cf) { genIncubatorModules(clb); // generate moduleDescriptors - genModuleDescriptorsMethod(clb, dedupSets); + genModuleDescriptorsMethod(clb); // generate moduleTargets genModuleTargetsMethod(clb); @@ -637,6 +635,9 @@ public byte[] genClassBytes(Configuration cf) { // generate moduleReads genModuleReads(clb, cf); + + // generate static initializer + genClassInitializer(clb); }); } @@ -655,20 +656,17 @@ private void genConstructor(ClassBuilder clb) { .return_()); } - private DedupSet genConstants(ClassBuilder clb) { - var dedupSet = dedupSetBuilder.build(clb); - var clinitSnippet = dedupSet.cacheSetupSnippet(); - if (!clinitSnippet.isEmpty()) { + private void genClassInitializer(ClassBuilder clb) { + if (!clinitSnippets.isEmpty()) { clb.withMethodBody( CLASS_INIT_NAME, MTD_void, ACC_STATIC, cob -> { - clinitSnippet.get().emit(cob); + clinitSnippets.forEach(s -> s.emit(cob)); cob.return_(); }); } - return dedupSet; } /** @@ -707,15 +705,19 @@ private void genIncubatorModules(ClassBuilder clb) { .ireturn()); } - private void genModuleDescriptorsMethod(ClassBuilder clb, DedupSet dedupSets) { + /** + * Generate bytecode for moduleDescriptors method + */ + private void genModuleDescriptorsMethod(ClassBuilder clb) { + var dedupSets = dedupSetBuilder.build(clb); + dedupSets.cacheSetupSnippet().ifPresent(clinitSnippets::add); + var converter = new ModuleDescriptorBuilder(clb, dedupSets, classDesc); var elementSnippets = converter.buildAll(moduleInfos); var moduleDescriptors = new ArraySnippetBuilder(CD_MODULE_DESCRIPTOR) .classBuilder(clb) - .activatePagingThreshold(moduleDescriptorsPerMethod) .ownerClassDesc(classDesc) - .methodNamePrefix("sub") - .pageSize(moduleDescriptorsPerMethod) + .enablePagination("sub", moduleDescriptorsPerMethod) .build(elementSnippets); clb.withMethodBody( @@ -869,6 +871,7 @@ private void generate(ClassBuilder clb, // map of Set -> local Map, Integer> locals; + int setBuilt = 0; // generate code to create the sets that are duplicated if (dedup) { @@ -880,7 +883,7 @@ private void generate(ClassBuilder clb, locals = new HashMap<>(); int index = 1; for (Set s : duplicateSets) { - genImmutableSet(cob, s); + genImmutableSet(clb, cob, s, methodName + setBuilt++); cob.astore(index); locals.put(s, index); if (++index >= MAX_LOCAL_VARS) { @@ -907,7 +910,7 @@ private void generate(ClassBuilder clb, // if de-duplicated then load the local, otherwise generate code Integer varIndex = locals.get(s); if (varIndex == null) { - genImmutableSet(cob, s); + genImmutableSet(clb, cob, s, methodName + setBuilt++); } else { cob.aload(varIndex); } @@ -932,9 +935,12 @@ private void generate(ClassBuilder clb, /** * Generate code to generate an immutable set. */ - private void genImmutableSet(CodeBuilder cob, Set set) { + private void genImmutableSet(ClassBuilder clb, CodeBuilder cob, Set set, String methodNamePrefix) { var snippets = Snippet.buildAll(sorted(set), Snippet::loadConstant); new SetSnippetBuilder(CD_String) + .classBuilder(clb) + .ownerClassDesc(classDesc) + .enablePagination(methodNamePrefix, STRING_PAGE_SIZE) .build(snippets) .emit(cob); } @@ -1032,7 +1038,9 @@ void hashForModule(String name, byte[] hash) { } /* - * Wraps set creation, ensuring identical sets are properly deduplicated. + * Snippets to load the deduplicated set onto the operand stack. + * Set referenced more than once will be read from the cache, cacheSetupSnippet contains + * the bytecode to populate that cache. */ static record DedupSet(Map, Snippet> stringSets, Map, Snippet> requiresModifiersSets, @@ -1040,6 +1048,9 @@ static record DedupSet(Map, Snippet> stringSets, Map, Snippet> exportsModifiersSets, Optional cacheSetupSnippet) {}; + /* + * Wraps set creation, ensuring identical sets are properly deduplicated. + */ static class DedupSetBuilder { final Map, SetReference> stringSets = new HashMap<>(); final Map, SetReference> @@ -1058,69 +1069,81 @@ static class DedupSetBuilder { } /* - * Add the given set of strings to this builder. - */ + * Add the given set of strings to this builder. + */ void stringSet(Set strings) { stringSets.computeIfAbsent(strings, SetReference::new).increment(); } /* - * Add the given set of Exports.Modifiers - */ + * Add the given set of Exports.Modifiers + */ void exportsModifiers(Set mods) { exportsModifiersSets.computeIfAbsent(mods, SetReference::new).increment(); } /* - * Add the given set of Opens.Modifiers - */ + * Add the given set of Opens.Modifiers + */ void opensModifiers(Set mods) { opensModifiersSets.computeIfAbsent(mods, SetReference::new).increment(); } /* - * Add the given set of Requires.Modifiers - */ + * Add the given set of Requires.Modifiers + */ void requiresModifiers(Set mods) { requiresModifiersSets.computeIfAbsent(mods, SetReference::new).increment(); } + /* + * Generate bytecode to load a set onto the operand stack. + * Use cache if the set is references more than once. + */ private Snippet buildStringSet(ClassBuilder clb, SetReference setRef) { return cacheBuilder.transform(setRef, new SetSnippetBuilder(CD_String) .classBuilder(clb) - .activatePagingThreshold(PAGING_THRESHOLD) .ownerClassDesc(owner) - .methodNamePrefix("dedupSet" + setBuilt++) - .pageSize(STRING_PAGE_SIZE) + .enablePagination("dedupSet" + setBuilt++, STRING_PAGE_SIZE) .build(Snippet.buildAll(setRef.elements(), Snippet::loadConstant))); } + /* + * Generate the mapping from a set to the bytecode loading the set onto the operand stack. + * Ordering the sets to ensure same generated bytecode. + */ + private Map, Snippet> buildStringSets(ClassBuilder clb, Map, SetReference> map) { + Map, Snippet> snippets = new HashMap<>(map.size()); + map.entrySet().stream() + .sorted(Map.Entry.comparingByValue()) + .forEach(e -> snippets.put(e.getKey(), buildStringSet(clb, e.getValue()))); + return snippets; + } + + /* + * Enum set support + */ private > Snippet buildEnumSet(ClassBuilder clb, SetReference setRef) { return cacheBuilder.transform(setRef, new SetSnippetBuilder(CD_Object) .classBuilder(clb) - .activatePagingThreshold(PAGING_THRESHOLD) .ownerClassDesc(owner) - .methodNamePrefix("dedupSet" + setBuilt++) - .pageSize(ENUM_PAGE_SIZE) + .enablePagination("dedupSet" + setBuilt++, ENUM_PAGE_SIZE) .build(Snippet.buildAll(setRef.elements(), Snippet::loadEnum))); } - private Map, Snippet> buildStringSets(ClassBuilder clb, Map, SetReference> map) { - Map, Snippet> snippets = new HashMap<>(map.size()); - map.entrySet().forEach(entry -> snippets.put(entry.getKey(), - buildStringSet(clb, entry.getValue()))); - return snippets; - } - private > Map, Snippet> buildEnumSets(ClassBuilder clb, Map, SetReference> map) { Map, Snippet> snippets = new HashMap<>(map.size()); - map.entrySet().forEach(entry -> snippets.put(entry.getKey(), - buildEnumSet(clb, entry.getValue()))); + map.entrySet().stream() + .sorted(Map.Entry.comparingByValue()) + .forEach(e -> snippets.put(e.getKey(), buildEnumSet(clb, e.getValue()))); return snippets; } + /* + * Build snippets for all sets and optionally the cache. + */ DedupSet build(ClassBuilder clb) { return new DedupSet( buildStringSets(clb, stringSets), @@ -1132,13 +1155,11 @@ DedupSet build(ClassBuilder clb) { } /* - * SetReference count references to the set, and use LoadableSet under the hood to - * support paginiation as needed. - * For sets referenced more than once, a cache is used to store the pre-built result - * and load from there. Otherwise, the set is built in place and load onto the operand - * stack. + * SetReference count references to the set, and keeps sorted elements to ensure + * generate same bytecode for a given set. + * SetReference itself needs ordering to ensure generate same bytecode for the cache. */ - class SetReference> { + class SetReference> implements Comparable> { // sorted elements of the set to ensure same generated code private final List elements; private int refCount; @@ -1158,8 +1179,32 @@ int refCount() { List elements() { return elements; } + + @Override + public int compareTo(SetReference o) { + if (o == this) { + return 0; + } + if (elements.size() == o.elements.size()) { + var a1 = elements; + var a2 = o.elements; + for (int i = 0; i < elements.size(); i++) { + var r = a1.get(i).compareTo(a2.get(i)); + if (r != 0) { + return r; + } + } + return 0; + } else { + return elements.size() - o.elements.size(); + } + } } + /** + * Build an array to host sets referenced more than once so a given set will only be constructed once. + * Transform the bytecode for loading the set onto the operand stack as needed. + */ class CacheBuilder { private static final String VALUES_ARRAY = "dedupSetValues"; final ArrayList cachedValues = new ArrayList<>(); @@ -1177,6 +1222,10 @@ private Snippet loadFromCache(int index) { .aaload(); } + /** + * Transform the bytecode for loading the set onto the operand stack. + * @param loadSnippet The origin snippet to load the set onto the operand stack. + */ Snippet transform(SetReference setRef, Snippet loadSnippet) { if (setRef.refCount() > 1) { cachedValues.add(loadSnippet); @@ -1187,22 +1236,20 @@ Snippet transform(SetReference setRef, Snippet loadSnippet) { } /* - * Adding provider methods to the class. For those set used more than once, built - * once and keep the reference for later access. - * Return a snippet to setup the cache . - * - * The returned snippet would set up the set referenced more than once, - * - * static final Set[] dedupSetValues; - * - * static { - * dedupSetValues = new Set[countOfStoredValues]; - * dedupSetValues[0] = Set.of(elements); // elements no more than SET_SIZE_THRESHOLD - * dedupSetValues[1] = dedupProvider(); // set elements more than SET_SIZE_THRESHOLD - * ... - * dedupSetValues[countOfStoredValues - 1] = ... - * } - */ + * Facilitate the cache in the class. Return a snippet to populate the cache in . + * + * The generated cache is essentially as the following: + * + * static final Set[] dedupSetValues; + * + * static { + * dedupSetValues = new Set[countOfStoredValues]; + * dedupSetValues[0] = Set.of(elements); // for inline set + * dedupSetValues[1] = dedupSet0(); // for paginated set + * ... + * dedupSetValues[countOfStoredValues - 1] = ... + * } + */ Optional build(ClassBuilder clb) { if (cachedValues.isEmpty()) { return Optional.empty(); @@ -1210,10 +1257,8 @@ Optional build(ClassBuilder clb) { var cacheValuesArray = new ArraySnippetBuilder(CD_Set) .classBuilder(clb) - .activatePagingThreshold(PAGING_THRESHOLD) .ownerClassDesc(owner) - .methodNamePrefix(VALUES_ARRAY) - .pageSize(2000) + .enablePagination(VALUES_ARRAY) .build(cachedValues.toArray(Snippet[]::new)); clb.withField(VALUES_ARRAY, CD_Set.arrayType(), ACC_STATIC | ACC_FINAL); diff --git a/test/jdk/tools/jlink/SnippetsTest.java b/test/jdk/tools/jlink/SnippetsTest.java index b2f3443bc138f..a6e802adb1912 100644 --- a/test/jdk/tools/jlink/SnippetsTest.java +++ b/test/jdk/tools/jlink/SnippetsTest.java @@ -118,19 +118,22 @@ void testArraySnippetBuilder() { var className = "LoadableArrayOf200Paged"; var elementSnippets = Snippet.buildAll(Arrays.asList(expected), Snippet::loadInteger); var instance = new ArraySnippetBuilder(CD_Integer) - .activatePagingThreshold(expected.length - 1) .ownerClassDesc(ClassDesc.of(className)) - .methodNamePrefix("page") - .pageSize(100); + .enablePagination("page", 100); + + try { + instance.build(elementSnippets); + fail("Should throw NPE without ClassBuilder"); + } catch (NullPointerException npe) { + // expected + } Supplier supplier = generateSupplier(className, clb -> instance.classBuilder(clb).build(elementSnippets)); verifyPaginationMethods(supplier.getClass(), Integer.class, "page", 2); assertArrayEquals(expected, supplier.get()); - var loadable = instance.activatePagingThreshold(expected.length) + var loadable = instance.disablePagination() .ownerClassDesc(ClassDesc.of("LoadableArrayOf200NotPaged")) - .methodNamePrefix("page") - .pageSize(100) .build(elementSnippets); // SimpleArray generate bytecode inline, so can be generated in any class @@ -162,10 +165,8 @@ void testSetSnippetBuilder() { var className = "AllSetTestPageNotActivated"; var methodNamePrefix = "page"; - var loadable = setBuilder.activatePagingThreshold(all.size()) + var loadable = setBuilder.disablePagination() .ownerClassDesc(ClassDesc.of(className)) - .methodNamePrefix(methodNamePrefix) - .pageSize(10) .build(allSnippets); supplier = generateSupplier(className, clb -> loadable); assertEquals(all, supplier.get()); @@ -173,8 +174,7 @@ void testSetSnippetBuilder() { className = "AllSetTestPageSize20"; setBuilder.ownerClassDesc(ClassDesc.of(className)); supplier = generateSupplier(className, clb -> setBuilder.classBuilder(clb) - .activatePagingThreshold(all.size() - 1) - .pageSize(20) + .enablePagination(methodNamePrefix, 20) .build(allSnippets)); verifyPaginationMethods(supplier.getClass(), String.class, methodNamePrefix, 5); assertEquals(all, supplier.get()); @@ -187,10 +187,8 @@ void testPaginatedArray(int elementCount, int pageSize) { var className = String.format("SnippetArrayProviderTest%dPagedBy%d", elementCount, pageSize); ClassDesc testClassDesc = ClassDesc.of(className); var builder = new ArraySnippetBuilder(CD_String) - .activatePagingThreshold(1) - .ownerClassDesc(testClassDesc) - .methodNamePrefix("ArrayPage") - .pageSize(pageSize); + .enablePagination("ArrayPage", pageSize, 1) + .ownerClassDesc(testClassDesc); var snippets = Snippet.buildAll(Arrays.asList(expected), Snippet::loadConstant); var pagingContext = new PagingContext(expected.length, pageSize); @@ -206,9 +204,11 @@ void testSimpleArray(int elementCount) { .toArray(String[]::new); String className = "SnippetArrayProviderTest" + elementCount; var array = new ArraySnippetBuilder(CD_String) + .disablePagination() .build(Snippet.buildAll(Arrays.asList(expected), Snippet::loadConstant)); Supplier supplier = generateSupplier(className, clb -> array); + verifyPaginationMethods(supplier.getClass(), String.class, "page", 0); assertArrayEquals(expected, supplier.get()); } From 8fc4e2f4c4d93d09dc4772a7d40d180ba74abcfd Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Sun, 22 Dec 2024 22:12:35 -0800 Subject: [PATCH 14/15] Address review comments --- .../jdk/tools/jlink/internal/Snippets.java | 29 ++++---- .../plugins/ModuleDescriptorBuilder.java | 20 +++--- .../internal/plugins/SystemModulesPlugin.java | 67 +++++++++---------- test/jdk/tools/jlink/JLink20000Packages.java | 5 +- test/jdk/tools/jlink/SnippetsTest.java | 6 +- 5 files changed, 64 insertions(+), 63 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java index 430f4007a258f..edddf3ce3f06e 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -93,7 +93,7 @@ static Snippet[] buildAll(Collection elements, Function fn) { } /** - * Describe a operand that can be load onto the operand stack. + * Describe an operand that can be load onto the operand stack. * For example, an array of string can be described as a Loadable. * * @param classDesc The type of the operand @@ -172,12 +172,12 @@ public int lastPageSize() { */ public static abstract class CollectionSnippetBuilder { /** - * Tested page size of string array + * Default page size of string array */ public static final int STRING_PAGE_SIZE = 8000; /** - * Tested page size of enum array + * Default page size of enum array */ public static final int ENUM_PAGE_SIZE = 5000; @@ -187,7 +187,7 @@ public static abstract class CollectionSnippetBuilder { public static final int DEFAULT_PAGE_SIZE = 2000; /** - * Arbitary default values based on 15K code size on ~30 bytes per element + * Default threshold based on 15K code size on ~30 bytes per element */ protected static final int DEFAULT_THRESHOLD = 512; @@ -195,9 +195,9 @@ public static abstract class CollectionSnippetBuilder { protected ClassDesc ownerClassDesc; protected ClassBuilder clb; - // Default values enable pagination by default - protected String methodNamePrefix = "csb" + Integer.toHexString(hashCode()) + "Page"; - protected int activatePagingThreshold = DEFAULT_THRESHOLD; + // Default values, disable pagination by default + protected String methodNamePrefix = null; + protected int activatePagingThreshold = -1; protected int pageSize = DEFAULT_PAGE_SIZE; /** @@ -275,6 +275,9 @@ public CollectionSnippetBuilder ownerClassDesc(ClassDesc ownerClassDesc) { */ public CollectionSnippetBuilder methodNamePrefix(String methodNamePrefix) { this.methodNamePrefix = Objects.requireNonNull(methodNamePrefix); + if (methodNamePrefix.isBlank()) { + throw new IllegalArgumentException(); + } return this; } @@ -304,7 +307,7 @@ public CollectionSnippetBuilder classBuilder(ClassBuilder clb) { } protected boolean shouldPaginate(int length) { - return activatePagingThreshold > 0 && length > activatePagingThreshold; + return methodNamePrefix != null && activatePagingThreshold > 0 && length > activatePagingThreshold; } /** @@ -332,10 +335,10 @@ protected boolean shouldPaginate(int length) { * Each pagination method will assign value to the corresponding page and chain calling next page. * * Effectively as - * methodNamePrefix0(new T[elements.size()]); + * methodNamePrefix_0(new T[elements.size()]); * * where - * T[] methodNamePrefix0(T[] ar) { + * T[] methodNamePrefix_0(T[] ar) { * ar[0] = elements[0]; * ar[1] = elements[1]; * ... @@ -369,7 +372,7 @@ private void invokePageHelper(CodeBuilder cob) { // Invoke the first page, which will call next page until fulfilled cob.loadConstant(loadElementSnippets.length) .anewarray(elementType) - .invokestatic(ownerClassDesc, methodNamePrefix + "0", MTD_PageHelper); + .invokestatic(ownerClassDesc, methodNamePrefix + "_0", MTD_PageHelper); } private void newArray(CodeBuilder cob) { @@ -396,7 +399,7 @@ private void setupHelpers() { private void genFillPageHelper(int pageNo, boolean hasNextPage) { var fromIndex = pageSize * pageNo; var toIndex = hasNextPage ? (fromIndex + pageSize) : loadElementSnippets.length; - clb.withMethodBody(methodNamePrefix + pageNo, + clb.withMethodBody(methodNamePrefix + "_" + pageNo, MTD_PageHelper, ACC_STATIC, cob -> { @@ -405,7 +408,7 @@ private void genFillPageHelper(int pageNo, boolean hasNextPage) { if (hasNextPage) { cob.invokestatic( ownerClassDesc, - methodNamePrefix + (pageNo + 1), + methodNamePrefix + "_" + (pageNo + 1), MTD_PageHelper); } cob.return_(TypeKind.from(classDesc)); diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java index a0d9cda16252a..dac50c861612e 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ModuleDescriptorBuilder.java @@ -45,7 +45,7 @@ import static jdk.tools.jlink.internal.Snippets.CollectionSnippetBuilder.STRING_PAGE_SIZE; import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.ModuleInfo; -import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.SystemModulesClassGenerator.DedupSet; +import jdk.tools.jlink.internal.plugins.SystemModulesPlugin.SystemModulesClassGenerator.DedupSnippets; /** * Build a Snippet to load a ModuleDescriptor onto the operand stack. @@ -56,13 +56,13 @@ class ModuleDescriptorBuilder implements IndexedElementSnippetBuilder { - dedupSet.requiresModifiersSets().get(require.modifiers()).emit(cob); + dedupSnippets.requiresModifiersSets().get(require.modifiers()).emit(cob); cob.loadConstant(require.name()); if (require.compiledVersion().isPresent()) { cob.loadConstant(require.compiledVersion().get().toString()) @@ -181,11 +181,11 @@ private Snippet buildRequiresArray(ClassBuilder clb) { */ Snippet loadExports(Exports export) { return cob -> { - dedupSet.exportsModifiersSets().get(export.modifiers()).emit(cob); + dedupSnippets.exportsModifiersSets().get(export.modifiers()).emit(cob); cob.loadConstant(export.source()); var targets = export.targets(); if (!targets.isEmpty()) { - dedupSet.stringSets().get(targets).emit(cob); + dedupSnippets.stringSets().get(targets).emit(cob); cob.invokestatic(CD_MODULE_BUILDER, "newExports", MTD_EXPORTS_MODIFIER_SET_STRING_SET); @@ -219,11 +219,11 @@ private Snippet buildExportsArray(ClassBuilder clb) { */ Snippet loadOpens(Opens open) { return cob -> { - dedupSet.opensModifiersSets().get(open.modifiers()).emit(cob); + dedupSnippets.opensModifiersSets().get(open.modifiers()).emit(cob); cob.loadConstant(open.source()); var targets = open.targets(); if (!targets.isEmpty()) { - dedupSet.stringSets().get(targets).emit(cob); + dedupSnippets.stringSets().get(targets).emit(cob); cob.invokestatic(CD_MODULE_BUILDER, "newOpens", MTD_OPENS_MODIFIER_SET_STRING_SET); @@ -327,7 +327,7 @@ public void emit(CodeBuilder cob) { MTD_OPENS_ARRAY); // uses - dedupSet.stringSets().get(md.uses()).emit(cob); + dedupSnippets.stringSets().get(md.uses()).emit(cob); cob.invokevirtual(CD_MODULE_BUILDER, "uses", MTD_SET); 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 0492a78fe396f..1c126d7321c24 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 @@ -1042,22 +1042,22 @@ void hashForModule(String name, byte[] hash) { * Set referenced more than once will be read from the cache, cacheSetupSnippet contains * the bytecode to populate that cache. */ - static record DedupSet(Map, Snippet> stringSets, - Map, Snippet> requiresModifiersSets, - Map, Snippet> opensModifiersSets, - Map, Snippet> exportsModifiersSets, - Optional cacheSetupSnippet) {}; + static record DedupSnippets(Map, Snippet> stringSets, + Map, Snippet> requiresModifiersSets, + Map, Snippet> opensModifiersSets, + Map, Snippet> exportsModifiersSets, + Optional cacheSetupSnippet) {}; /* * Wraps set creation, ensuring identical sets are properly deduplicated. */ static class DedupSetBuilder { - final Map, SetReference> stringSets = new HashMap<>(); - final Map, SetReference> + final Map, RefCounter> stringSets = new HashMap<>(); + final Map, RefCounter> requiresModifiersSets = new HashMap<>(); - final Map, SetReference> + final Map, RefCounter> exportsModifiersSets = new HashMap<>(); - final Map, SetReference> + final Map, RefCounter> opensModifiersSets = new HashMap<>(); final ClassDesc owner; @@ -1072,48 +1072,48 @@ static class DedupSetBuilder { * Add the given set of strings to this builder. */ void stringSet(Set strings) { - stringSets.computeIfAbsent(strings, SetReference::new).increment(); + stringSets.computeIfAbsent(strings, RefCounter::new).increment(); } /* * Add the given set of Exports.Modifiers */ void exportsModifiers(Set mods) { - exportsModifiersSets.computeIfAbsent(mods, SetReference::new).increment(); + exportsModifiersSets.computeIfAbsent(mods, RefCounter::new).increment(); } /* * Add the given set of Opens.Modifiers */ void opensModifiers(Set mods) { - opensModifiersSets.computeIfAbsent(mods, SetReference::new).increment(); + opensModifiersSets.computeIfAbsent(mods, RefCounter::new).increment(); } /* * Add the given set of Requires.Modifiers */ void requiresModifiers(Set mods) { - requiresModifiersSets.computeIfAbsent(mods, SetReference::new).increment(); + requiresModifiersSets.computeIfAbsent(mods, RefCounter::new).increment(); } /* * Generate bytecode to load a set onto the operand stack. - * Use cache if the set is references more than once. + * Use cache if the set is referenced more than once. */ - private Snippet buildStringSet(ClassBuilder clb, SetReference setRef) { + private Snippet buildStringSet(ClassBuilder clb, RefCounter setRef) { return cacheBuilder.transform(setRef, new SetSnippetBuilder(CD_String) .classBuilder(clb) .ownerClassDesc(owner) .enablePagination("dedupSet" + setBuilt++, STRING_PAGE_SIZE) - .build(Snippet.buildAll(setRef.elements(), Snippet::loadConstant))); + .build(Snippet.buildAll(setRef.sortedList(), Snippet::loadConstant))); } /* * Generate the mapping from a set to the bytecode loading the set onto the operand stack. * Ordering the sets to ensure same generated bytecode. */ - private Map, Snippet> buildStringSets(ClassBuilder clb, Map, SetReference> map) { + private Map, Snippet> buildStringSets(ClassBuilder clb, Map, RefCounter> map) { Map, Snippet> snippets = new HashMap<>(map.size()); map.entrySet().stream() .sorted(Map.Entry.comparingByValue()) @@ -1124,16 +1124,16 @@ private Map, Snippet> buildStringSets(ClassBuilder clb, Map> Snippet buildEnumSet(ClassBuilder clb, SetReference setRef) { + private > Snippet buildEnumSet(ClassBuilder clb, RefCounter setRef) { return cacheBuilder.transform(setRef, new SetSnippetBuilder(CD_Object) .classBuilder(clb) .ownerClassDesc(owner) .enablePagination("dedupSet" + setBuilt++, ENUM_PAGE_SIZE) - .build(Snippet.buildAll(setRef.elements(), Snippet::loadEnum))); + .build(Snippet.buildAll(setRef.sortedList(), Snippet::loadEnum))); } - private > Map, Snippet> buildEnumSets(ClassBuilder clb, Map, SetReference> map) { + private > Map, Snippet> buildEnumSets(ClassBuilder clb, Map, RefCounter> map) { Map, Snippet> snippets = new HashMap<>(map.size()); map.entrySet().stream() .sorted(Map.Entry.comparingByValue()) @@ -1144,8 +1144,8 @@ private > Map, Snippet> buildEnumSets(ClassBuilder clb, /* * Build snippets for all sets and optionally the cache. */ - DedupSet build(ClassBuilder clb) { - return new DedupSet( + DedupSnippets build(ClassBuilder clb) { + return new DedupSnippets( buildStringSets(clb, stringSets), buildEnumSets(clb, requiresModifiersSets), buildEnumSets(clb, opensModifiersSets), @@ -1155,16 +1155,16 @@ DedupSet build(ClassBuilder clb) { } /* - * SetReference count references to the set, and keeps sorted elements to ensure + * RefCounter count references to the set, and keeps sorted elements to ensure * generate same bytecode for a given set. - * SetReference itself needs ordering to ensure generate same bytecode for the cache. + * RefCounter itself needs ordering to ensure generate same bytecode for the cache. */ - class SetReference> implements Comparable> { + class RefCounter> implements Comparable> { // sorted elements of the set to ensure same generated code private final List elements; private int refCount; - SetReference(Set elements) { + RefCounter(Set elements) { this.elements = sorted(elements); } @@ -1176,12 +1176,12 @@ int refCount() { return refCount; } - List elements() { + List sortedList() { return elements; } @Override - public int compareTo(SetReference o) { + public int compareTo(RefCounter o) { if (o == this) { return 0; } @@ -1209,11 +1209,8 @@ class CacheBuilder { private static final String VALUES_ARRAY = "dedupSetValues"; final ArrayList cachedValues = new ArrayList<>(); - // Load the set to the operand stack. - // When referenced more than once, the value is pre-built with static initialzer - // and is load from the cache array with + // Load the set from the cache to the operand stack // dedupSetValues[index] - // Otherwise, LoadableSet will load the set onto the operand stack. private Snippet loadFromCache(int index) { assert index >= 0; return cob -> @@ -1226,7 +1223,7 @@ private Snippet loadFromCache(int index) { * Transform the bytecode for loading the set onto the operand stack. * @param loadSnippet The origin snippet to load the set onto the operand stack. */ - Snippet transform(SetReference setRef, Snippet loadSnippet) { + Snippet transform(RefCounter setRef, Snippet loadSnippet) { if (setRef.refCount() > 1) { cachedValues.add(loadSnippet); return loadFromCache(cachedValues.size() - 1); @@ -1236,7 +1233,7 @@ Snippet transform(SetReference setRef, Snippet loadSnippet) { } /* - * Facilitate the cache in the class. Return a snippet to populate the cache in . + * Returns a snippet that populates the cached values in . * * The generated cache is essentially as the following: * @@ -1245,7 +1242,7 @@ Snippet transform(SetReference setRef, Snippet loadSnippet) { * static { * dedupSetValues = new Set[countOfStoredValues]; * dedupSetValues[0] = Set.of(elements); // for inline set - * dedupSetValues[1] = dedupSet0(); // for paginated set + * dedupSetValues[1] = dedupSet_0(); // for paginated set * ... * dedupSetValues[countOfStoredValues - 1] = ... * } diff --git a/test/jdk/tools/jlink/JLink20000Packages.java b/test/jdk/tools/jlink/JLink20000Packages.java index ee24283e00858..865cf7ca98c2f 100644 --- a/test/jdk/tools/jlink/JLink20000Packages.java +++ b/test/jdk/tools/jlink/JLink20000Packages.java @@ -62,6 +62,7 @@ static void javac(String[] args) { public static void main(String[] args) throws Exception { Path src = Paths.get("bug8321413"); + Path imageDir = src.resolve("out-jlink"); Path mainModulePath = src.resolve("bug8321413x"); StringJoiner mainModuleInfoContent = new StringJoiner(";\n exports ", "module bug8321413x {\n exports ", ";\n}"); @@ -106,12 +107,12 @@ public static void main(String[] args) throws Exception { JImageGenerator.getJLinkTask() .modulePath(out) - .output(src.resolve("out-jlink")) + .output(imageDir) .addMods("bug8321413x") .call() .assertSuccess(); - Path binDir = src.resolve("out-jlink").resolve("bin").toAbsolutePath(); + Path binDir = imageDir.resolve("bin").toAbsolutePath(); Path bin = binDir.resolve("java"); ProcessBuilder processBuilder = new ProcessBuilder(bin.toString(), diff --git a/test/jdk/tools/jlink/SnippetsTest.java b/test/jdk/tools/jlink/SnippetsTest.java index a6e802adb1912..4029a860adbdc 100644 --- a/test/jdk/tools/jlink/SnippetsTest.java +++ b/test/jdk/tools/jlink/SnippetsTest.java @@ -61,7 +61,7 @@ * @run junit SnippetsTest */ public class SnippetsTest { - private static final boolean WRITE_CLASS_FILE = Boolean.parseBoolean(System.getProperty("DumpArraySnippetsTestClasses", "true")); + private static final boolean WRITE_CLASS_FILE = Boolean.parseBoolean(System.getProperty("DumpArraySnippetsTestClasses", "false")); @ParameterizedTest @ValueSource(ints = { 10, 75, 90, 120, 200, 399, 400, 401}) @@ -229,14 +229,14 @@ void verifyPaginationMethods(Class testClass, Class elementType, String me var methodType = MethodType.methodType(elementType.arrayType(), elementType.arrayType()); if (pageCount <= 0) { try { - lookup().findStatic(testClass, methodNamePrefix + 0, methodType); + lookup().findStatic(testClass, methodNamePrefix + "_0", methodType); fail("Unexpected paginate helper function"); } catch (Exception ex) {} } for (int i = 0; i < pageCount; i++) { try { - lookup().findStatic(testClass, methodNamePrefix + i, methodType); + lookup().findStatic(testClass, methodNamePrefix + "_" + i, methodType); } catch (Exception ex) { throw new RuntimeException(ex); } From 760f66728053d212b8e60b6db5e57998499fd3e8 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Fri, 3 Jan 2025 08:51:15 -0800 Subject: [PATCH 15/15] Clean up --- .../share/classes/jdk/tools/jlink/internal/Snippets.java | 4 ++-- test/jdk/tools/jlink/SnippetsTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java index edddf3ce3f06e..d662e297f9a6b 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java @@ -343,7 +343,7 @@ protected boolean shouldPaginate(int length) { * ar[1] = elements[1]; * ... * ar[pageSize-1] = elements[pageSize - 1]; - * methodNamePrefix1(ar); + * methodNamePrefix_1(ar); * return ar; * } * and the last page will stop the chain and can be partial instead of full page size. @@ -394,7 +394,7 @@ private void setupHelpers() { } } - // each helper function is T[] methodNamePrefix{pageNo}(T[]) + // each helper function is T[] methodNamePrefix_{pageNo}(T[]) // fill the page portion and chain calling to fill next page private void genFillPageHelper(int pageNo, boolean hasNextPage) { var fromIndex = pageSize * pageNo; diff --git a/test/jdk/tools/jlink/SnippetsTest.java b/test/jdk/tools/jlink/SnippetsTest.java index 4029a860adbdc..ec0c94e3d264b 100644 --- a/test/jdk/tools/jlink/SnippetsTest.java +++ b/test/jdk/tools/jlink/SnippetsTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 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