From d53e405a26e53086d46ce78a9792f0ca72cca529 Mon Sep 17 00:00:00 2001 From: Claes Redestad Date: Mon, 9 Sep 2024 14:18:20 +0000 Subject: [PATCH] 8339742: Refactor ClassFileImpl to allow loading Option classes lazily Reviewed-by: asotona --- .../classfile/impl/AbstractDirectBuilder.java | 2 +- .../classfile/impl/ClassFileImpl.java | 138 ++++++++++++++---- .../classfile/impl/ClassReaderImpl.java | 2 +- .../jdk/internal/classfile/impl/CodeImpl.java | 4 +- .../classfile/impl/DirectCodeBuilder.java | 43 +++--- .../classfile/impl/StackMapGenerator.java | 6 +- .../jdk/internal/classfile/impl/Util.java | 4 +- 7 files changed, 135 insertions(+), 64 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java b/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java index f574627491f..6b75a44a6bf 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java @@ -51,7 +51,7 @@ public class AbstractDirectBuilder { } public void writeAttribute(Attribute a) { - if (Util.isAttributeAllowed(a, context.attributesProcessingOption())) { + if (Util.isAttributeAllowed(a, context)) { attributes.withAttribute(a); } } diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassFileImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassFileImpl.java index 1fd4c5cb1a0..c6d9e55d8db 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassFileImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassFileImpl.java @@ -31,7 +31,6 @@ import java.util.function.Consumer; import java.lang.classfile.AttributeMapper; import java.lang.classfile.ClassFile; -import java.lang.classfile.ClassFile.*; import java.lang.classfile.ClassBuilder; import java.lang.classfile.ClassHierarchyResolver; import java.lang.classfile.ClassModel; @@ -41,33 +40,53 @@ import java.lang.classfile.constantpool.ConstantPoolBuilder; import java.lang.classfile.constantpool.Utf8Entry; import jdk.internal.classfile.impl.verifier.VerifierImpl; -public record ClassFileImpl(StackMapsOption stackMapsOption, - DebugElementsOption debugElementsOption, - LineNumbersOption lineNumbersOption, - AttributesProcessingOption attributesProcessingOption, - ConstantPoolSharingOption constantPoolSharingOption, - ShortJumpsOption shortJumpsOption, - DeadCodeOption deadCodeOption, - DeadLabelsOption deadLabelsOption, - ClassHierarchyResolverOption classHierarchyResolverOption, - AttributeMapperOption attributeMapperOption) implements ClassFile { +public final class ClassFileImpl implements ClassFile { + + private Option stackMapsOption; + private Option debugElementsOption; + private Option lineNumbersOption; + private Option attributesProcessingOption; + private Option constantPoolSharingOption; + private Option shortJumpsOption; + private Option deadCodeOption; + private Option deadLabelsOption; + private Option classHierarchyResolverOption; + private Option attributeMapperOption; + + private ClassFileImpl(Option stackMapsOption, + Option debugElementsOption, + Option lineNumbersOption, + Option attributesProcessingOption, + Option constantPoolSharingOption, + Option shortJumpsOption, + Option deadCodeOption, + Option deadLabelsOption, + Option classHierarchyResolverOption, + Option attributeMapperOption) { + this.stackMapsOption = stackMapsOption; + this.debugElementsOption = debugElementsOption; + this.lineNumbersOption = lineNumbersOption; + this.attributesProcessingOption = attributesProcessingOption; + this.constantPoolSharingOption = constantPoolSharingOption; + this.shortJumpsOption = shortJumpsOption; + this.deadCodeOption = deadCodeOption; + this.deadLabelsOption = deadLabelsOption; + this.classHierarchyResolverOption = classHierarchyResolverOption; + this.attributeMapperOption = attributeMapperOption; + } public static final ClassFileImpl DEFAULT_CONTEXT = new ClassFileImpl( - StackMapsOption.STACK_MAPS_WHEN_REQUIRED, - DebugElementsOption.PASS_DEBUG, - LineNumbersOption.PASS_LINE_NUMBERS, - AttributesProcessingOption.PASS_ALL_ATTRIBUTES, - ConstantPoolSharingOption.SHARED_POOL, - ShortJumpsOption.FIX_SHORT_JUMPS, - DeadCodeOption.PATCH_DEAD_CODE, - DeadLabelsOption.FAIL_ON_DEAD_LABELS, - new ClassHierarchyResolverOptionImpl(ClassHierarchyResolver.defaultResolver()), - new AttributeMapperOptionImpl(new Function<>() { - @Override - public AttributeMapper apply(Utf8Entry k) { - return null; - } - })); + null, // StackMapsOption.STACK_MAPS_WHEN_REQUIRED + null, // DebugElementsOption.PASS_DEBUG, + null, // LineNumbersOption.PASS_LINE_NUMBERS, + null, // AttributesProcessingOption.PASS_ALL_ATTRIBUTES, + null, // ConstantPoolSharingOption.SHARED_POOL, + null, // ShortJumpsOption.FIX_SHORT_JUMPS, + null, // DeadCodeOption.PATCH_DEAD_CODE, + null, // DeadLabelsOption.FAIL_ON_DEAD_LABELS, + null, // new ClassHierarchyResolverOptionImpl(ClassHierarchyResolver.defaultResolver()), + null // _ -> null + ); @SuppressWarnings("unchecked") @Override @@ -85,6 +104,8 @@ public record ClassFileImpl(StackMapsOption stackMapsOption, for (var o : options) { if (o instanceof StackMapsOption oo) { smo = oo; + } else if (o instanceof ClassHierarchyResolverOption oo) { + chro = oo; } else if (o instanceof DebugElementsOption oo) { deo = oo; } else if (o instanceof LineNumbersOption oo) { @@ -99,8 +120,6 @@ public record ClassFileImpl(StackMapsOption stackMapsOption, dco = oo; } else if (o instanceof DeadLabelsOption oo) { dlo = oo; - } else if (o instanceof ClassHierarchyResolverOption oo) { - chro = oo; } else if (o instanceof AttributeMapperOption oo) { amo = oo; } else { // null or unknown Option type @@ -127,9 +146,8 @@ public record ClassFileImpl(StackMapsOption stackMapsOption, @Override public byte[] transformClass(ClassModel model, ClassEntry newClassName, ClassTransform transform) { - ConstantPoolBuilder constantPool = constantPoolSharingOption() == ConstantPoolSharingOption.SHARED_POOL - ? ConstantPoolBuilder.of(model) - : ConstantPoolBuilder.of(); + ConstantPoolBuilder constantPool = sharedConstantPool() ? ConstantPoolBuilder.of(model) + : ConstantPoolBuilder.of(); return build(newClassName, constantPool, new Consumer() { @Override @@ -141,10 +159,14 @@ public record ClassFileImpl(StackMapsOption stackMapsOption, }); } + public boolean sharedConstantPool() { + return constantPoolSharingOption == null || constantPoolSharingOption == ConstantPoolSharingOption.SHARED_POOL; + } + @Override public List verify(ClassModel model) { try { - return VerifierImpl.verify(model, classHierarchyResolverOption().classHierarchyResolver(), null); + return VerifierImpl.verify(model, classHierarchyResolver(), null); } catch (IllegalArgumentException verifierInitializationError) { return List.of(new VerifyError(verifierInitializationError.getMessage())); } @@ -159,6 +181,58 @@ public record ClassFileImpl(StackMapsOption stackMapsOption, } } + public Function> attributeMapper() { + if (attributeMapperOption == null) { + return _ -> null; + } else { + return ((AttributeMapperOption)attributeMapperOption).attributeMapper(); + } + } + + public ClassHierarchyResolver classHierarchyResolver() { + if (classHierarchyResolverOption == null) { + return ClassHierarchyImpl.DEFAULT_RESOLVER; + } else { + return ((ClassHierarchyResolverOption)classHierarchyResolverOption).classHierarchyResolver(); + } + } + + public boolean dropDeadLabels() { + return (deadLabelsOption != null && deadLabelsOption == DeadLabelsOption.DROP_DEAD_LABELS); + } + + public boolean passDebugElements() { + return (debugElementsOption == null || debugElementsOption == DebugElementsOption.PASS_DEBUG); + } + + public boolean passLineNumbers() { + return (lineNumbersOption == null || lineNumbersOption == LineNumbersOption.PASS_LINE_NUMBERS); + } + + public AttributesProcessingOption attributesProcessingOption() { + return (attributesProcessingOption == null) ? AttributesProcessingOption.PASS_ALL_ATTRIBUTES : (AttributesProcessingOption)attributesProcessingOption; + } + + public boolean fixShortJumps() { + return (shortJumpsOption == null || shortJumpsOption == ShortJumpsOption.FIX_SHORT_JUMPS); + } + + public boolean stackMapsWhenRequired() { + return (stackMapsOption == null || stackMapsOption == StackMapsOption.STACK_MAPS_WHEN_REQUIRED); + } + + public boolean generateStackMaps() { + return (stackMapsOption == StackMapsOption.GENERATE_STACK_MAPS); + } + + public boolean dropStackMaps() { + return (stackMapsOption == StackMapsOption.DROP_STACK_MAPS); + } + + public boolean patchDeadCode() { + return (deadCodeOption == null || deadCodeOption == DeadCodeOption.PATCH_DEAD_CODE); + } + public record AttributeMapperOptionImpl(Function> attributeMapper) implements AttributeMapperOption { } diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassReaderImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassReaderImpl.java index 9695b16ad51..6a4b0cd486f 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassReaderImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassReaderImpl.java @@ -82,7 +82,7 @@ public final class ClassReaderImpl this.buffer = classfileBytes; this.classfileLength = classfileBytes.length; this.context = context; - this.attributeMapper = this.context.attributeMapperOption().attributeMapper(); + this.attributeMapper = this.context.attributeMapper(); if (classfileLength < 4 || readInt(0) != 0xCAFEBABE) { throw new IllegalArgumentException("Bad magic number"); } diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/CodeImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/CodeImpl.java index 3ea8f377f3c..e08d2b76479 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/CodeImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/CodeImpl.java @@ -122,7 +122,7 @@ public final class CodeImpl if (!inflated) { if (labels == null) labels = new LabelImpl[codeLength + 1]; - if (classReader.context().lineNumbersOption() == ClassFile.LineNumbersOption.PASS_LINE_NUMBERS) + if (classReader.context().passLineNumbers()) inflateLineNumbers(); inflateJumpTargets(); inflateTypeAnnotations(); @@ -167,7 +167,7 @@ public final class CodeImpl inflateMetadata(); boolean doLineNumbers = (lineNumbers != null); generateCatchTargets(consumer); - if (classReader.context().debugElementsOption() == ClassFile.DebugElementsOption.PASS_DEBUG) + if (classReader.context().passDebugElements()) generateDebugElements(consumer); for (int pos=codeStart; pos a = new UnboundAttribute.AdHocAttribute<>(Attributes.characterRangeTable()) { @@ -232,7 +232,7 @@ public final class DirectCodeBuilder var start = labelToBci(cr.startScope()); var end = labelToBci(cr.endScope()); if (start == -1 || end == -1) { - if (context.deadLabelsOption() == ClassFile.DeadLabelsOption.DROP_DEAD_LABELS) { + if (context.dropDeadLabels()) { crSize--; } else { throw new IllegalArgumentException("Unbound label in character range"); @@ -261,7 +261,7 @@ public final class DirectCodeBuilder b.writeU2(lvSize); for (LocalVariable l : localVariables) { if (!Util.writeLocalVariable(b, l)) { - if (context.deadLabelsOption() == ClassFile.DeadLabelsOption.DROP_DEAD_LABELS) { + if (context.dropDeadLabels()) { lvSize--; } else { throw new IllegalArgumentException("Unbound label in local variable type"); @@ -284,7 +284,7 @@ public final class DirectCodeBuilder b.writeU2(localVariableTypes.size()); for (LocalVariableType l : localVariableTypes) { if (!Util.writeLocalVariable(b, l)) { - if (context.deadLabelsOption() == ClassFile.DeadLabelsOption.DROP_DEAD_LABELS) { + if (context.dropDeadLabels()) { lvtSize--; } else { throw new IllegalArgumentException("Unbound label in local variable type"); @@ -357,24 +357,21 @@ public final class DirectCodeBuilder } if (codeAndExceptionsMatch(codeLength)) { - switch (context.stackMapsOption()) { - case STACK_MAPS_WHEN_REQUIRED -> { - attributes.withAttribute(original.findAttribute(Attributes.stackMapTable()).orElse(null)); - writeCounters(true, buf); - } - case GENERATE_STACK_MAPS -> - generateStackMaps(buf); - case DROP_STACK_MAPS -> - writeCounters(true, buf); + if (context.stackMapsWhenRequired()) { + attributes.withAttribute(original.findAttribute(Attributes.stackMapTable()).orElse(null)); + writeCounters(true, buf); + } else if (context.generateStackMaps()) { + generateStackMaps(buf); + } else if (context.dropStackMaps()) { + writeCounters(true, buf); } } else { - switch (context.stackMapsOption()) { - case STACK_MAPS_WHEN_REQUIRED -> - tryGenerateStackMaps(false, buf); - case GENERATE_STACK_MAPS -> - generateStackMaps(buf); - case DROP_STACK_MAPS -> - writeCounters(false, buf); + if (context.stackMapsWhenRequired()) { + tryGenerateStackMaps(false, buf); + } else if (context.generateStackMaps()) { + generateStackMaps(buf); + } else if (context.dropStackMaps()) { + writeCounters(false, buf); } } diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java b/src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java index 5825eb2f547..c6564c289d6 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java @@ -235,9 +235,9 @@ public final class StackMapGenerator { this.labelContext = labelContext; this.handlers = handlers; this.rawHandlers = new ArrayList<>(handlers.size()); - this.classHierarchy = new ClassHierarchyImpl(context.classHierarchyResolverOption().classHierarchyResolver()); - this.patchDeadCode = context.deadCodeOption() == ClassFile.DeadCodeOption.PATCH_DEAD_CODE; - this.filterDeadLabels = context.deadLabelsOption() == ClassFile.DeadLabelsOption.DROP_DEAD_LABELS; + this.classHierarchy = new ClassHierarchyImpl(context.classHierarchyResolver()); + this.patchDeadCode = context.patchDeadCode(); + this.filterDeadLabels = context.dropDeadLabels(); this.currentFrame = new Frame(classHierarchy); generate(); } diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/Util.java b/src/java.base/share/classes/jdk/internal/classfile/impl/Util.java index d4d91d9aab1..b0f1de65d92 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/Util.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/Util.java @@ -102,9 +102,9 @@ public class Util { private static final int ATTRIBUTE_STABILITY_COUNT = AttributeMapper.AttributeStability.values().length; public static boolean isAttributeAllowed(final Attribute attr, - final ClassFile.AttributesProcessingOption processingOption) { + final ClassFileImpl context) { return attr instanceof BoundAttribute - ? ATTRIBUTE_STABILITY_COUNT - attr.attributeMapper().stability().ordinal() > processingOption.ordinal() + ? ATTRIBUTE_STABILITY_COUNT - attr.attributeMapper().stability().ordinal() > context.attributesProcessingOption().ordinal() : true; }