Openj9: Account HCR for OpenJDK MethodHandles

[babsingh]

HCR - Hot Code Replacement

For OpenJDK MethodHandles, an instance of MemberName stores:

• MemberName->vmtarget (J9Method)
• MemberName->vmindex (J9JNIMethodID)

The above J9Method and J9JNIMethodID references can become outdated due to HCR. They need to be kept updated.

https://github.com/eclipse/openj9/pull/11367#discussion_r545422877 [Babneet Singh]

1. Should VM_VMHelpers::currentClass get the most current version of class after HCR?

2. Do we also need to update the J9Method ref stored in MemberName for HCR? Related code:

   • hshelp.c::fixDirectHandles: Here J9Methods stored in OpenJ9 MHs are updated. We probably need to do the same for MemberNames->vmtarget (J9Method).
   • hshelp.c::fixJNIRefs will need to be invoked before fixing MemberNames->vmindex (J9JNIMethodID).
   • jvmtiClass.c::redefineClassesCommon: invokes hshelp.c::fixJNIRefs and hshelp.c::fixDirectHandles.
   • Related question: https://github.com/eclipse/openj9/pull/10690#discussion_r531728225

https://github.com/eclipse/openj9/pull/11367#discussion_r545531656 [Dan Heidinga]

Should VM_VMHelpers::currentClass get the most current version of class in linkToInterface after HCR?

This solves half the problem in that it ensures the most up-to-date version of the class is used in the itable search. Unfortunately, the itableIndex may not point to the same method in the new version of the class if the methods have been reordered.

It's really two pieces of data - class version & itable index - and both need to kept consistent and up to date.

hshelp.c::fixDirectHandles: Here J9Methods stored in OpenJ9 MHs are updated. We probably need to do the same for MemberNames->vmtarget (J9Method).

This is carefully set up thru cooperation between the MH creation java code & the redef code to avoid a full heap walk to find the DirectHandles. We may be able to make the same invariants hold for MemberName but I'm less clear on how to do that without patching the OJDK java code.

Couple of options here:

1. Do a full heap walk on redefinition to fix the vmtarget & vmindex fields of the MemberName. This will regress the use cases we fixed by adding the DirectHandle cache but it will be correct.
2. Change vmindex to be an index into class->jniIDs table. This should let the existing mechanisms fix the JNIID when the class is redefined but will require changes to the way invocation occurs. We'll also need to figure out how to update vmtarget as well.
3. Something else?

I'd suggest starting with option 1 as it's the easiest to get right and we can figure out how to optimize it later.

[babsingh]

The VM_VMHelpers::currentClass change is handled in https://github.com/eclipse/openj9/pull/11367. The bigger change will be handled in a separate PR.

[babsingh]

fyi @DanHeidinga @tajila @fengxue-IS

[liqunl]

Two questions:

1. Can methods generated for MethodHandle be redefined?
2. Are vmtarget and vmindex declared as final fields in MemberName?

[babsingh]

@liqunl

Can methods generated for MethodHandle be redefined?

Yes.

Are vmtarget and vmindex declared as final fields in MemberName?

We inject those fields during runtime. They are not final.

@fengxue-IS for confirmation.

[liqunl]

We inject those fields during runtime. They are not final

The JIT treats them as final fields. If they can change, the JIT needs to know about it or detect their change for functional correctness.

[liqunl]

For methods generated for MethodHandle, my understanding is that they're not visible to the user, so user can't and shouldn't redefine them. How can the user get hold of those methods and redefine them? Those methods are critical to MethodHandle implementation, why they are allowed to be redefined?

[babsingh]

I am not familiar with HCR.

++ @gacholio @DanHeidinga @tajila To help address @liqunl's above concerns: https://github.com/eclipse/openj9/issues/11528#issuecomment-759521610 and https://github.com/eclipse/openj9/issues/11528#issuecomment-759523644.

[tajila]

Can methods generated for MethodHandle be redefined?

Yes.

Will methods generated for MHs always be in hidden/anon classes?

I cant recall if those are allowed to be redefined, the rules have changed in the past. We currently have a comment indicating that it is not possible

/*
 * Can't replace:
 * - primitive or array classes
 * - J9VMInternals
 * - sun.misc.Unsafe Anonymous classes
 * - Object (in extended HCR mode)
 */
jboolean
classIsModifiable(J9JavaVM * vm, J9Class * clazz)
{
    return !J9ROMCLASS_IS_UNMODIFIABLE(clazz->romClass);
}

[tajila]

It looks like from Java 11+ they are unmodifiable

        if (NULL != _javaVM) {
            if ((J2SE_VERSION(_javaVM) >= J2SE_V11) 
                && (isClassAnon() || isClassHidden())
            ) {
                unmodifiable = true;
            } else if (NULL == J9VMJAVALANGOBJECT_OR_NULL(_javaVM)) {
                /* Object is currently only allowed to be redefined in fast HCR */
                if (areExtensionsEnabled(_javaVM)) {
                    unmodifiable = true;
                }
            }
        }

[babsingh]

Will methods generated for MHs always be in hidden/anon classes?

Yes. References:

1. JDK16 (hidden) InvokerBytecodeGenerator.java#L318
2. JDK11 (anon) InvokerBytecodeGenerator.java#L287
3. JDK8 (anon) InvokerBytecodeGenerator.java#L276

For JDK8, anon classes will be modifiable. Should we look into making OJDK MH generated classes unmodifiable for JDK8; instead of adding HCR support?

[gacholio]

We should make them unmodifiable if we continue to pass all the required tests.

If we are not able to make them unmodifiable, I suggest we do nothing and let the VM crash if someone is foolish enough to attempt this. I suspect no customer will ever report this.

[babsingh]

We should make them unmodifiable if we continue to pass all the required tests.

All OJDK MH/VH generated anon classes should have LambdaForm as the HOST_CLASS. Also, LambdaForm (as host class) should not be used for anything other than OJDK MH/VH. For JDK8, we can probably use (isAnon && (hostClass == LambdaForm)) as a filter to make OJDK MH/VH generated classes unmodifiable.

[tajila]

We had an issue regarding this last year, https://github.com/eclipse/openj9/issues/10504, https://github.com/eclipse/openj9/issues/7354 and https://github.com/eclipse/openj9/pull/10483. Initially redefinition of anonClasses were disabled but had to be re-enabled to pass tests.

[babsingh]

Update: MemberName->vmtarget/vmindex can point to user Java methods for some of the INL methods. This will be the case for direct method handles. For such cases, we would need to update the vmindex and vmtarget in MemberNames after HCR. _We will ignore issues arising from this for the time being and keep it in the back-burner._ After HCR, the MemberName->vmtarget/vmindex will be updated by the VM so the JIT won't need any special handling. For the initial solution, the stale pointers will be updated by walking the full heap.