From 69d84664fa4feeb27fdaa5d1d6ba407724becf9f Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Thu, 14 Nov 2024 22:31:51 -0800 Subject: [PATCH] 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(); }); });