RFR: 8328111: Convert Runtime tests to use the ClassFile API instead of ASM [v20]

[Chen Liang]

On Mon, 1 Apr 2024 13:00:03 GMT, Oussama Louati <duke at openjdk.org> wrote:

>> The main goal pull request migrate the existing tests to utilize the new ClassFile API instead of the ASM library. The use of ASM has served us well in the past, but maintaining it has its costs and limitations.
>> 
>> ### **Cost of Maintaining ASM Library:**
>> 
>> - _Risk of Changes:_ The ASM library is an external dependency, and updates or changes in its APIs can introduce unforeseen issues or require significant code modifications in our tests.
>> - _Lack of Reviewers:_ As the ASM library evolves, finding reviewers familiar with its intricacies becomes challenging, leading to delays in reviewing and merging changes.
>> 
>> ### **Reasons for Migration:**
>> 
>> - _Stability and Consistency:_ Utilizing the ClassFile API ensures stability and compatibility with the Java platform, reducing the risk of compatibility issues due to external library changes.
>> - _Sustainability:_ The ClassFile API is a standard part of Java, making it easier to find reviewers and maintain code consistency across test suites.
>> - _Future Compatibility:_ As Java evolves, relying on native Java APIs like the ClassFile API ensures future compatibility and reduces technical debt.
>> 
>> ### **Concerns Addressed:**
>> 
>> - Risk of Changes: By migrating to a standard Java API, we mitigate the risks associated with external library changes and ensure smoother maintenance and updates in the future.
>> - Backporting and Compatibility: The use of native Java APIs allows for easier backporting of test fixes and ensures compatibility across Java versions, including preview releases.
>> 
>>> **Please note:** We won't integrate this PR until we've fully finalized the ClassFile API and all tests utilize it.
>
> Oussama Louati has updated the pull request incrementally with one additional commit since the last revision:
> 
>   resolve review comments

test/hotspot/jtreg/runtime/ConstantPool/BadMethodHandles.java line 50:

> 48:     private static final ClassDesc CD_System = ClassDesc.of("java.lang.System");
> 49:     private static final ClassDesc CD_PrintStream = ClassDesc.of("java.io.PrintStream");
> 50:     private static final ClassDesc CD_LPrintStream = ClassDesc.ofDescriptor("Ljava/io/PrintStream;");

This is the same as `CD_PrintStream`, so no need for two constants.

test/hotspot/jtreg/runtime/ConstantPool/BadMethodHandles.java line 154:

> 152: 
> 153:         DirectMethodHandleDesc handle = MethodHandleDesc.of(DirectMethodHandleDesc.Kind.VIRTUAL,
> 154:                 ClassDesc.of("InvokeBasicref"), "runInvokeBasicM", "()V");

Suggestion:

                CD_MethodHandle, "invokeBasic", "([Ljava/lang/Object;)Ljava/lang/Object;");

test/hotspot/jtreg/runtime/ConstantPool/IntfMethod.java line 72:

> 70:                                 cob -> cob
> 71:                                         .aload(0)
> 72:                                         .invokespecial(cob.constantPool().interfaceMethodRefEntry(ClassDesc.of("C"), "f1", MTD_void))

Suggestion:

                                        .invokespecial(ClassDesc.of("C"), "f1", MTD_void, true)

test/hotspot/jtreg/runtime/ConstantPool/IntfMethod.java line 78:

> 76:                         .withMethodBody("testStaticClass", MTD_void, ACC_PUBLIC,
> 77:                                 cob -> cob
> 78:                                         .invokestatic(cob.constantPool().interfaceMethodRefEntry(ClassDesc.of("C"), "f2", MTD_void))

Suggestion:

                                        .invokestatic(ClassDesc.of("C"), "f2", MTD_void, true)

test/hotspot/jtreg/runtime/StackTrace/LargeClassTest.java line 91:

> 89:                                             .pop()
> 90:                                             .return_());
> 91:                     for (int i = 1000; i < 34560 ; i++) {

Suggestion:

                    // Write 33560 methods called f_$i
                    for (int i = 1000; i < 34560; i++) {

test/hotspot/jtreg/runtime/finalStatic/FinalStatic.java line 104:

> 102:                     break;
> 103:             }
> 104:             return null;

Suggestion:

            throw new AssertionError("Unknown class " + name);

test/hotspot/jtreg/runtime/invokedynamic/BootstrapMethodErrorTest.java line 58:

> 56:         @Override
> 57:         public Class findClass(String name) throws ClassNotFoundException {
> 58:             byte[] b = new byte[0];

Suggestion:

            byte[] b;

test/hotspot/jtreg/runtime/verifier/TestANewArray.java line 110:

> 108:         classCName = "classCName_" + cfv + "_" + testDimension264;
> 109: 
> 110:         return ClassFile.of(ClassFile.StackMapsOption.DROP_STACK_MAPS).build(ClassDesc.of(classCName),

Suggestion:

        return ClassFile.of().build(ClassDesc.of(classCName),

The original ClassWriter has `COMPUTE_FRAMES` flag.

test/hotspot/jtreg/runtime/verifier/TestANewArray.java line 123:

> 121:                                     cob -> cob
> 122:                                             .bipush(1)
> 123:                                             .anewarray(cob.constantPool().classEntry(cob.constantPool().utf8Entry(arrayType)))

Suggestion:

                                            .anewarray(ClassDesc.ofDescriptor(arrayType))

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617465372
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617502358
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617512949
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617513886
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617518305
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617524484
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617525117
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617531980
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617535574