JEP-370 Implementation (Part 2)

[babsingh]

Related: #8292.

This pull request has commits from and is dependent on #8330 and #8369.

The top seven commits are new, and they are described below. Files changed will show all the commits. So, individually refer to the new commits in order to review the changes specific to this pull request.

1. Implement LambdaForm.BasicType.basicType(Class<?> cls)

BasicType.basicType takes a class as input and outputs the corresponding
BasicType enum value depending on the class's basic type char. The
class's basic type char is derived using the sun.invoke.util.Wrapper
class.

2. Implement the constructor MemberName(Method method)

The method passed in the constructor, MemberName(Method method), is
cached in the MemberName instance.

3. Initialize AccessMode.methodNameToAccessMode (HashMap)

AccessMode.methodNameToAccessMode is a HashMap<String, AccessMode>,
where the key is the method name and the value is the AccessMode enum
value. This hashmap is used in AccessMode.valueFromMethodName to
retrieve the AccessMode enum value for the corresponding method name. It
is also directly accessed from VarForm.linkFromStatic. It is needed so
that OpenJ9 can consume OpenJDK's VarForm class.

4. Implement constructor VarHandle(VarForm varForm)

In this constructor, fieldType, coordinateTypes, handleTable and
modifiers are initialized using the VarForm class. This should allow us
to consume OpenJDK's VarHandles with OpenJ9's VarHandle class as the
base class. Currently, it will only be used for OpenJDK's MemoryAddress
VarHandles.

5. Handle conversion from OpenJDK to OpenJ9 VarHandle operation methods

OpenJDK VarHandle operation methods have the following parameter
sequence: {VarHandle, Receiver, Intermediate ..., Value}.

During invocation, OpenJ9 uses the following parameter sequence for the
VarHandle operations: {Receiver, Intermediate ..., Value, VarHandle}.

The location of the VarHandle argument is different in the above two
cases. MethodHandles.permuteArguments is used to translate from the
OpenJ9 to OpenJDK invocation.

6. Implement VarHandle.AccessType.accessModeType

AccessType.accessModeType is implemented to retrieve the receiver class
from the derived VarHandle class since VarForm does not provide the
exact receiver class. In VarForm, the receiver class is always
java.lang.Object.

7. Initialize VarHandle.handleTable with exact method types

When the AccessMode methods have the receiver class defined generically
or as java.lang.Object in the derived VarHandle classes, then the
handleTable needs to be initialized with the exact method types, which
will be used during invocation of the AccessMode methods. The exact
method types need to specify the actual receiver class. This change
yields correct behavior with respect to WrongMethodTypeException and
ClassCastException. Without this change, ClassCastException is thrown
when WrongMethodTypeException is expected.

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

[babsingh]

How to run JEP370 tests?

1. Get openj9-openjdk-jdk14 source with Allow GENSRC_VARHANDLES in OpenJ9 ibmruntimes/openj9-openjdk-jdk14#6:

git clone https://github.com/ibmruntimes/openj9-openjdk-jdk14.git

cd openj9-openjdk-jdk14
git remote add babsingh https://github.com/babsingh/openj9-openjdk-jdk14.git
git fetch babsingh
git cherry-pick afff6adc4a6f10a88a0afd0dd1ce6f9fe0c378c2

2. Get the boot JDK (OpenJDK13):

wget https://github.com/AdoptOpenJDK/openjdk13-binaries/releases/download/jdk-13.0.2%2B8_openj9-0.18.0/OpenJDK13U-jdk_x64_linux_openj9_13.0.2_8_openj9-0.18.0.tar.gz
tar -xvzf OpenJDK13U-jdk_x64_linux_openj9_13.0.2_8_openj9-0.18.0.tar.gz
mv jdk-13* bootjdk13
export JAVA_HOME=$PWD/bootjdk13

3. Get the JTReg test harness:

wget https://ci.adoptopenjdk.net/job/jtreg/lastSuccessfulBuild/artifact/jtreg-4.2-b16.tar.gz
tar -xvzf jtreg-4.2-b16.tar.gz

4. Get OpenJ9 and OMR source:

bash ./get_source.sh

cd openj9
git remote add babsingh 
git fetch babsingh https://github.com/babsingh/openj9.git
git checkout -b jep370_impl1 babsingh/jep370_impl1
cd ..

5. Get freemarker.jar:

wget https://sourceforge.net/projects/freemarker/files/freemarker/2.3.8/freemarker-2.3.8.tar.gz/download -O freemarker.tgz
tar -xzf freemarker.tgz freemarker-2.3.8/lib/freemarker.jar --strip=2
rm -f freemarker.tgz

6. Run configure with JTReg test harness:

bash ./configure --with-freemarker-jar=$PWD/freemarker.jar --with-jtreg=$PWD/jtreg

7. Build OpenJDK14:

make images

8. Run JEP370 tests:

make test-jdk_foreign |& tee jep370_tests.log

[babsingh]

JEP370 Tests Summary

Log output after running the JEP370 tests: jep370_tests8.log

There are two kinds of failures, which are seen repeatedly:

1. WrongMethodTypeException:

java.lang.invoke.WrongMethodTypeException: No conversion from '(MemoryAddress,short,VarHandle)void' to '(VarHandleMemoryAddressBase,Object,short)void at index (2)
        at java.base/java.lang.invoke.ConvertHandle.throwWrongMethodTypeException(ConvertHandle.java:119)
        at java.base/java.lang.invoke.ConvertHandle.checkConversion(ConvertHandle.java:104)
        at java.base/java.lang.invoke.ArgumentConversionHandle.<init>(ArgumentConversionHandle.java:31)
        at java.base/java.lang.invoke.AsTypeHandle.<init>(AsTypeHandle.java:31)
        at java.base/java.lang.invoke.MethodHandle.asType(MethodHandle.java:497)
        at java.base/java.lang.invoke.MethodHandle.forGenericInvoke(MethodHandle.java:1281)
        at TestVarHandleCombinators.testByteOrderLE(TestVarHandleCombinators.java:131)

Test source code: TestVarHandleCombinators.testByteOrderLE

    public void testByteOrderLE() {
        VarHandle vh = MemoryHandles.varHandle(short.class, 2, ByteOrder.LITTLE_ENDIAN);
        byte[] arr = new byte[2];
        MemorySegment segment = MemorySegment.ofArray(arr);
        MemoryAddress address = segment.baseAddress();

        vh.set(address, (short) 0xFF);
        assertEquals(arr[0], (byte) 0xFF);
        assertEquals(arr[1], (byte) 0);
    }

WrongMethodTypeException occurs while executing vh.set(...) (VarHandle method).

This is related to the difference in operations class. In OpenJ9, the VarHandle argument is the last argument. In OpenJDK, the VarHandle argument is the first argument. I updated few dependencies in order to account for both scenarios. I will need help in identifying the missed dependencies. @DanHeidinga @tajila

2. ClassCastException:

java.lang.ClassCastException: java.lang.String incompatible with java.lang.Class
        at java.base/java.lang.invoke.VarHandleMemoryAddressAsInts0/0000000000000000.carrier(Unknown Source)
        at java.base/java.lang.invoke.MethodHandleImpl$1.memoryAddressCarrier(MethodHandleImpl.java:1802)
        at jdk.incubator.foreign/jdk.incubator.foreign.MemoryHandles.withOffset(MemoryHandles.java:208)
        at TestVarHandleCombinators.testNestedSequenceAccess(TestVarHandleCombinators.java:156)

The MemoryAddress VarHandle classes are generated dynamically during runtime with ASM. class AddressVarHandleGenerator generates the MemoryAddress VarHandle classes. It adds an accessor method to access the carrier field of the VarHandle: AddressVarHandleGenerator.addCarrierAccessor. It relies upon constant pool patching. I am getting the above error with and without Jack's constant pool changes. Also, I noticed no difference in class bytecodes with and without Jack's constant pool changes: cppatching.zip *.class files. Is constant pool patching supposed to avoid the above error? @fengxue-IS

[tajila]

For WrongMethodTypeException what is the methodType for the "set" MH in the VarHandle handleTable?

Also, what is the methodType for the "set" MH in the in the VarForm methodType_table?

[babsingh]

For WrongMethodTypeException what is the methodType for the "set" MH in the VarHandle handleTable?

(VarHandle, Receiver, <Intermediates>, Value)void.

Also, what is the methodType for the "set" MH in the in the VarForm methodType_table?

(Receiver, <Intermediates>, Value)void.

VarForm methodType_table stores AccessType method signatures.

handleTable stores AccessMode method signatures, which are derived from VarForm.memberName_table.

In VarHandle.accessModeType, we drop the VarHandle argument from handleTable[accessMode].type, which should translate to the method type in VarForm methodType_table.

[babsingh]

VM_MHInterpreter::dispatchLoop has the VarHandle argument hardcoded as the last argument when invoking the VarHandle operation methods. @tajila suggested to to use MethodHandles.permuteArguments to resolve the WrongMethodTypeException. Commit 5 reflects the new change, and it resolves the WrongMethodTypeException.

JEP370 has 14 test-suites, which comprise of 95 test-cases in total. With this pull request, 85 test-cases pass and 10 test-cases fail. The latest log output after running the JEP370 tests: jep370_tests9.log.

There are two unique failures in the 10 failing test-cases:

1. 6 test cases fail because of AssertionError:

test TestByteBuffer.testScopedBufferAndVarHandle(java.lang.invoke.ByteBufferViewVarHandle@e0ad053e): failure
java.lang.AssertionError: null
	at org.testng.Assert.fail(Assert.java:94)
	at org.testng.Assert.fail(Assert.java:101)
	at TestByteBuffer.testScopedBufferAndVarHandle(TestByteBuffer.java:320)

Test source code: TestByteBuffer.testScopedBufferAndVarHandle. We need to throw UnsupportedOperationException for unsupported AccessModes when using OpenJDK VarHandle operations classes. This should fix the above 6 failures.

2. 4 test cases fail because of ClassCastException:

java.lang.ClassCastException: java.lang.String incompatible with java.lang.Class
	at java.base/java.lang.invoke.VarHandleMemoryAddressAsInts0/0000000000000000.carrier(Unknown Source)
	at java.base/java.lang.invoke.MethodHandleImpl$1.memoryAddressCarrier(MethodHandleImpl.java:1802)
	at jdk.incubator.foreign/jdk.incubator.foreign.MemoryHandles.withOffset(MemoryHandles.java:208)
	at TestVarHandleCombinators.testNestedSequenceAccess(TestVarHandleCombinators.java:156)

java.lang.ClassCastException: java.lang.Integer incompatible with jdk.internal.access.foreign.MemoryAddressProxy
	at java.base/java.lang.invoke.VarHandleMemoryAddressAsInts.checkAddress(VarHandleMemoryAddressAsInts.java:49)
	at java.base/java.lang.invoke.VarHandleMemoryAddressAsInts.get0(VarHandleMemoryAddressAsInts.java:76)
	at java.base/java.lang.invoke.VarHandleMemoryAddressAsInts0/0000000000000000.get(Unknown Source)
	at TestTypeAccess.testMemoryCoordinatePrimitive(TestTypeAccess.java:51)

I feel that these failures are related to something missing in constant pool patching. @fengxue-IS can you verify?

[fengxue-IS]

Will test this locally to verify cpPatching related issues

[babsingh]

Update: @fengxue-IS fixed constant pool patching to resolve ClassCastException: java.lang.Integer incompatible with jdk.internal.access.foreign.MemoryAddressProxy.

Two pending issues:

1. TestTypeAccess.testMemoryCoordinatePrimitive expects a WrongMethodTypeException but OpenJ9 throws a ClassCastException. INT_HANDLE.get's signature is int get(Object) (Object -> Receiver). In testMemoryCoordinatePrimitive INT_HANDLE.get(1), 1 is seen as an Integer (Object). This invokes the get operation method, and causes the ClassCastException instead of the WrongMethodTypeException.

java.lang.ClassCastException: java.lang.Integer incompatible with jdk.internal.access.foreign.MemoryAddressProxy
	at java.base/java.lang.invoke.VarHandleMemoryAddressAsInts.checkAddress(VarHandleMemoryAddressAsInts.java:49)
	at java.base/java.lang.invoke.VarHandleMemoryAddressAsInts.get0(VarHandleMemoryAddressAsInts.java:76)
	at java.base/java.lang.invoke.VarHandleMemoryAddressAsInts0/0000000000000000.get(Unknown Source)
	at TestTypeAccess.testMemoryCoordinatePrimitive(TestTypeAccess.java:51)

2. In TestByteBuffer.testScopedBufferAndVarHandle, accessing the MemorySegment outside the try (MemorySegment ...) { } scope should lead to IllegalStateException from MemoryScope. In OpenJ9, IllegalStateException is not thrown during handle.invoke.

test TestByteBuffer.testScopedBufferAndVarHandle(java.lang.invoke.ByteBufferViewVarHandle@e0ad053e): failure
java.lang.AssertionError: null
	at org.testng.Assert.fail(Assert.java:94)
	at org.testng.Assert.fail(Assert.java:101)
	at TestByteBuffer.testScopedBufferAndVarHandle(TestByteBuffer.java:320)

fyi @DanHeidinga @tajila

[babsingh]

Update:

In the header description, the new commits 6 and 7 resolve the following issue:

TestTypeAccess.testMemoryCoordinatePrimitive expects a WrongMethodTypeException but OpenJ9 throws a ClassCastException.

Only one kind of failure remains:

In TestByteBuffer.testScopedBufferAndVarHandle, accessing the MemorySegment outside the try (MemorySegment ...) { } scope should lead to IllegalStateException from MemoryScope. In OpenJ9, IllegalStateException is not thrown during handle.invoke.

@DanHeidinga I feel confident about this direction for JEP370. @fengxue-IS's constant pool changes work and only one kind of failure remains. Instead of working on the alternate solution, I will prioritize fixing the one remaining failure? Meanwhile, we can start merging ibmruntimes/openj9-openjdk-jdk14#6, #8330, @fengxue-IS's constant pool changes and this pull request? I will account for the one remaining failure in a separate pull request.

[DanHeidinga]

I feel confident about this direction for JEP370. @fengxue-IS's constant pool changes work and only one kind of failure remains. Instead of working on the alternate solution, I will prioritize fixing the one remaining failure?

+1 this approach.

[babsingh]

Depends on #8330 and #8460. Will require a rebase after dependents are merged.

[DanHeidinga]

This looks OK to me.

We'll need to figure out the delivery plan for this and parts 1 & 3 + @fengxue-IS's cp patch PRs

[babsingh]

Delivery plan:

D1. #8330 (Part 1) depends on ibmruntimes/openj9-openjdk-jdk14#6
D1. ibmruntimes/openj9-openjdk-jdk14#6 depends on #8330
D2. #8460 (CP String patching) independent
D3. #8414 (Part 2) depends on #8330 (Part 1); rebase required
D4. #8501 (Part 3) depends on #8330 (Part 1) and #8414 (Part 2); rebase required

The changes can be delivered in the above order. The first two items need to be delivered together.

[DanHeidinga]

@babsingh can you rebase this? The prereqs have been merged now

[babsingh]

@DanHeidinga rebased.

[DanHeidinga]

Jenkins test sanity zlinux jdk11,jdk14

[DanHeidinga]

@babsingh Can you open a PR for this against the 0.19.0 release branch?

[DanHeidinga]

And also rebase #8501 so it's ready for review?