RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v6]

liach duke at openjdk.org
Tue Mar 14 17:02:44 UTC 2023


On Tue, 14 Mar 2023 15:27:50 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> jdk.jlink internal plugins are heavily using ASM
>> 
>> This patch converts ASM calls to Classfile API.
>> 
>> Please review.
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - long lines wrapped
>  - StripJavaDebugAttributesPlugin transformation fixed
>  - VersionPropsPlugin transformation fixed

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 614:

> 612:         private void genConstructor(ClassBuilder clb) {
> 613:             clb.withMethod("<init>", MethodTypeDesc.of(CD_void),
> 614:                     ACC_PUBLIC, mb -> mb.withFlags(ACC_PUBLIC).withCode( cob -> {

Why `withFlags(ACC_PUBLIC)` when the flags is already given in `withMethod`'s 3rd argument? Same for occurrences below.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 615:

> 613:             clb.withMethod("<init>", MethodTypeDesc.of(CD_void),
> 614:                     ACC_PUBLIC, mb -> mb.withFlags(ACC_PUBLIC).withCode( cob -> {
> 615:                         cob.loadInstruction(TypeKind.ReferenceType, 0);

What's the basis for using `loadInstruction(ReferenceType, 0)` over `aload(0)`, etc.? I think, at least for constructors, we won't have other TypeKinds, so we can safely use `aload(0)` `return_()`, and this is what the original ASM code used.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 894:

> 892:             // use Set.of(Object[]) when there are more than 2 elements
> 893:             // use Set.of(Object) or Set.of(Object, Object) when fewer
> 894:             if (size > 2) {

Set.of can take up to 10 arguments, but this may be off topic.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 916:

> 914:                     sb.append("Ljava/lang/Object;");
> 915:                 }
> 916:                 sb.append(")Ljava/util/Set;");

The construction of the Set.of method type should move away from StringBuilder. It can be as:

var mtdArgs = new ClassDesc[size];
Arrays.fill(mtdArgs, CD_Object);
MethodTypeDesc mtd = MethodTypeDesc.of(CD_Set, mtdArgs);

-------------

PR: https://git.openjdk.org/jdk/pull/12944


More information about the core-libs-dev mailing list