Account HCR for OpenJDK MethodHandles #11528

babsingh · 2020-12-21T17:30:24Z

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.

#11367 (comment) [Babneet Singh]

Should VM_VMHelpers::currentClass get the most current version of class after HCR?
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: Implement MethodHandleNatives native code #10690 (comment)

#11367 (comment) [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:

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.
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.
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.

The text was updated successfully, but these errors were encountered:

babsingh · 2020-12-21T17:46:35Z

The VM_VMHelpers::currentClass change is handled in #11367. The bigger change will be handled in a separate PR.

babsingh · 2020-12-21T17:47:04Z

fyi @DanHeidinga @tajila @fengxue-IS

liqunl · 2021-01-13T15:17:24Z

Two questions:

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

babsingh · 2021-01-13T15:23:48Z

@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 · 2021-01-13T15:26:31Z

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 · 2021-01-13T15:29:34Z

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 · 2021-01-13T15:36:24Z

I am not familiar with HCR.

++ @gacholio @DanHeidinga @tajila To help address @liqunl's above concerns: #11528 (comment) and #11528 (comment).

tajila · 2021-01-13T15:41:31Z

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 · 2021-01-13T15:43:54Z

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 · 2021-01-13T15:59:04Z

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

Yes. References:

JDK16 (hidden) InvokerBytecodeGenerator.java#L318
JDK11 (anon) InvokerBytecodeGenerator.java#L287
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 · 2021-01-13T16:22:39Z

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 · 2021-01-13T16:45:37Z

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 · 2021-01-13T16:50:27Z

We had an issue regarding this last year, #10504, #7354 and #10483. Initially redefinition of anonClasses were disabled but had to be re-enabled to pass tests.

babsingh · 2021-01-14T19:56:02Z

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.

babsingh · 2021-04-01T13:34:58Z

In #12306 and #12304, SharedCPEntryInvokerTests_1 and SharedCPEntryInvokerTests_2 seg fault while redefining classes in the PR builds. These tests pass locally, and the failure couldn't be reproduced in a local debug environment.

Unhandled exception
Type=Segmentation error vmState=0x00040000
J9Generic_Signal_Number=00000018 Signal_Number=0000000b Error_Value=00000000 Signal_Code=00000001
Handler1=00007F52F89C0EB0 Handler2=00007F52F871FAC0 InaccessibleAddress=0000000000000034
RDI=00007F52780043A0 RSI=00007F52B95D20F0 RAX=0000000000000004 RBX=0000000000000030
RCX=0000000000000003 RDX=0000000000000030 R8=0000000000000001 R9=000000000003FA90
R10=0000000000000001 R11=00007F52F1C374D0 R12=0000000000172500 R13=0000000000172500
R14=000000000003FA90 R15=00007F52F4024590
RIP=00007F52F0FDFAA5 GS=0000 FS=0000 RSP=00007F52B95D20D0
EFlags=0000000000010206 CS=0033 RBP=0000000000000001 ERR=0000000000000004
TRAPNO=000000000000000E OLDMASK=0000000000000000 CR2=0000000000000034
xmm0 0000000000000000 (f: 0.000000, d: 0.000000e+00)
xmm1 6e65706f2f67726f (f: 795308672.000000, d: 6.199766e+223)
xmm2 0000000000000000 (f: 0.000000, d: 0.000000e+00)
xmm3 0700086f6f662e32 (f: 1868967424.000000, d: 5.788518e-275)
xmm4 6c75616665441300 (f: 1698960128.000000, d: 2.879083e+214)
xmm5 746e492f72656b6f (f: 1919249280.000000, d: 6.938835e+252)
xmm6 000000003b9a0000 (f: 999948288.000000, d: 4.940401e-315)
xmm7 0000000000000000 (f: 0.000000, d: 0.000000e+00)
xmm8 6f766e692f747365 (f: 796160896.000000, d: 8.502194e+228)
xmm9 0000000000000000 (f: 0.000000, d: 0.000000e+00)
xmm10 000000003e8c05e2 (f: 1049363968.000000, d: 5.184547e-315)
xmm11 000000003c075284 (f: 1007112832.000000, d: 4.975799e-315)
xmm12 0000000040400000 (f: 1077936128.000000, d: 5.325712e-315)
xmm13 0000000000000000 (f: 0.000000, d: 0.000000e+00)
xmm14 0000000000000000 (f: 0.000000, d: 0.000000e+00)
xmm15 0000000000000000 (f: 0.000000, d: 0.000000e+00)
Module=/home/jenkins/workspace/Test_openjdk16_j9_sanity.functional_x86-64_linux_ojdk292_Personal_testList_0/openjdkbinary/j2sdk-image/lib/default/libj9jvmti29.so
Module_base_address=00007F52F0FAE000
Target=2_90_20210326_4 (Linux 4.4.0-190-generic)
CPU=amd64 (4 logical CPUs) (0x5e2f01000 RAM)
----------- Stack Backtrace -----------
(0x00007F52F0FDFAA5 [libj9jvmti29.so+0x31aa5])
(0x00007F52F0FB4BD7 [libj9jvmti29.so+0x6bd7])
(0x00007F52F0FB7304 [libj9jvmti29.so+0x9304])
Java_com_ibm_jvmti_tests_redefineClasses_rc001_redefineClass+0x81 (0x00007F52BBAC25B1 [libjvmtitest.so+0x1a5b1])
(0x00007F52F8B35306 [libj9vm29.so+0x1b3306])
(0x00007F52F8B342A7 [libj9vm29.so+0x1b22a7])
(0x00007F52F8A6C3D4 [libj9vm29.so+0xea3d4])
(0x00007F52F8A4C26D [libj9vm29.so+0xca26d])
(0x00007F52F8AC3652 [libj9vm29.so+0x141652])

EricYangIBM · 2021-05-26T14:07:34Z

#11367 (comment) [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.

@DanHeidinga
I am unsure how the iTableIndex's may change. According to this, HCR does not change the signature of a class, only its method bodies. If the class signature doesn't change (and methods are not reordered), the old iTableIndex's should index to the same methods.

DanHeidinga · 2021-05-27T14:49:47Z

@EricYangIBM good question. And welcome to having to understand the poorly named "HCR" feature(s) in openj9. We refer to HCR in a very loose way to mean class redefinition but there are two different modes of HCR:

fastHCR which is spec compliant and keeps as much of the pre-existing J9Class structure stable as possible during a redefinition
FSD or full speed debug which allows the use of J9-specific extensions to add or remove methods. FSD recreates J9Classs rather than keep them stable so more is expected to change.

For the fastHCR case, @gacholio fixed the itable indexes to be stable in #5397 though the direct method cases where we've tagged the itable index to cover direct calls and calls to methods defined in Object may need to be addressed still. Kind of depends on how they get resolved - as interface methods or as direct sends? - in the OJDK MH implementation.

For the FSD case, the expedient solution is a full heap walk to find all the MHs and fix up their fields. We've done more creative approaches in the past but that required some cooperation between the MH implementation and the HCR code so they may not be possible with the OJDK MHs.

tldr; this is mostly not a problem for fastHCR but you'll need to validate the cases of specially tagged itable indexes. FSD will need a heap walk if itables are redefined.

fengxue-IS · 2021-05-27T20:01:45Z

The information that MH store related to a J9Class is

Class object: MemberName.declaringClass
jniFieldID / jniMethodID: MemberName.vmindex (hidden field)
fieldOffset / J9Method* / vTableIndex: MemberName.vmtarget (hidden field)

With a closer look at how the data is stored, I am not sure if clearing the invokeCache table would be sufficient as other classes could have MemberName references into the J9Class being redefined.

So for HCR, since we can keep the old J9Class* 's address then we don't have to worry about (1)
For 2/3 I am not sure if we can use similar approach where we swap the data in the structure but keep the pointer address
If we can't then need to walk the entire heap's memberName object and check if the vmindex field is set and is pointing to the class being redefined. Then update the vmindex/vmtarget to correct value by generating a mapping between old/new jniIDs

~~@DanHeidinga @gacholio would it be possible for us to only swap the structure data but keep the pointer address for jniIDs and J9Methods?~~

@nbhuiyan how would this affect jit code for MethodHandle? does JIT already have check for redefinition and regenerate the compiled code?

fengxue-IS · 2021-05-27T21:02:39Z

Actually, nvm for the above on keeping pointer address, I checked the code for fixJNIRefs it seems that JNI ID pointers are already fixed, so we can do a heap walk to fix all memberName objects with declaring class matching the redefined class

DanHeidinga · 2021-05-28T01:01:27Z

So for HCR, since we can keep the old J9Class* 's address then we don't have to worry about (1)

HCR is an ambiguous term. We need to be more specific here as the above is true for fastHCR but wrong for FSD.

nbhuiyan · 2021-05-28T02:01:16Z

@fengxue-IS

how would this affect jit code for MethodHandle? does JIT already have check for redefinition and regenerate the compiled code?

As far as I know we don't have a mechanism in the JIT to check for MH class redefinitions. If MemberName.vmtarget and MemberName.vmindex gets updated after HCR, then there should be no need for the JIT to regenerate anything.

fengxue-IS · 2021-05-28T19:27:57Z

So for HCR, since we can keep the old J9Class* 's address then we don't have to worry about (1)

HCR is an ambiguous term. We need to be more specific here as the above is true for fastHCR but wrong for FSD.

Thanks for the correction Dan, I didn't think about FSD here.

For FSD, I think we also need to update the class object stored in MemberName.clazz
Given how jniIDs are fixed:

 * Always invalidate the old field ID slot (do not free IDs for deleted fields, since a reflect field might be holding onto it)
 * If the old field was not deleted, move the fieldID to the new class

 * if equivalent and old exists, copy old methodID to new method
 * if equivalent and old doesn't exists, create methodID for old method copy to new method
 * if non-equivalent and old exists, copy old methodID to new method, and create a new methodID for the old chain
 * if non-equivalent and old does not exist, do nothing
 * if new doesn't exist do nothing

@DanHeidinga @gacholio In the case that field/method does not exist in new class, should we still allow MemberName to reference the old ID? if not, what value should the vmindex/vmtarget be set to?

gacholio · 2021-05-31T15:58:45Z

Will using the old ID continue to function is some sensible way? Adding and removing methods is an OpenJ9 extension, so there's no spec for this case.

The other option is to put an invalid value like UDATA_MAX in there. The issue then becomes detecting the invalid value at all of the appropriate places. If extended FSD HCR is used, I believe we flush the JIT code cache, so there's no need to put the check in compiled code.

EricYangIBM · 2021-05-31T17:24:18Z

For FSD, I think we also need to update the class object stored in MemberName.clazz

Does the call to fixHeapRefs in redefineClassesCommon fix these class refs? Specifically

openj9/runtime/util/hshelp.c

Lines 2058 to 2081 in b2b6f49

    
           static jvmtiIterationControl 
        
           fixHeapRefsObjectIteratorCallback(J9JavaVM *vm, J9MM_IterateObjectDescriptor *objectDesc, void *userData) 
        
           { 
        
           	J9HashTable* classHashTable = userData; 
        
           	j9object_t object = objectDesc->object; 
        
           	J9Class *clazz = J9OBJECT_CLAZZ_VM(vm, object); 
        
           	if (NULL != classHashTable) { 
        
           		J9JVMTIClassPair exemplar; 
        
           		J9JVMTIClassPair* result = NULL; 
        
           		exemplar.originalRAMClass = clazz; 
        
           		result = hashTableFind(classHashTable, &exemplar); 
        
           		if (NULL != result) { 
        
           			J9Class * replacementRAMClass = result->replacementClass.ramClass; 
        
           			if (NULL != replacementRAMClass) { 
        
           				J9OBJECT_SET_CLAZZ_VM(vm, objectDesc->object, replacementRAMClass); 
        
           			} 
        
           		} 
        
           	} 
        
           	return JVMTI_ITERATION_CONTINUE; 
        
           }

gacholio · 2021-05-31T17:48:47Z

No, that function fixes the classes of heap objects.

EricYangIBM · 2021-05-31T19:11:34Z

@gacholio For FSD, what fixes the heap classObjects? I.e. the Class<?> references.

gacholio · 2021-05-31T19:15:24Z

Assuming you mean what fixes up the java.lang.Class references to the J9Class: copyPreservedValues does it - search for J9VMJAVALANGCLASS_SET_VMREF.

EricYangIBM · 2021-06-01T13:47:31Z

~~> Assuming you mean what fixes up the java.lang.Class references to the J9Class: copyPreservedValues does it - search for J9VMJAVALANGCLASS_SET_VMREF.~~

@gacholio Looking at copyPreservedValues, it seems like it only sets the class reference of the replacement class. What I'm wondering about is if the class references as fields get updated at all. E.g. an unrelated java object has a Class field that is pointing to an old class, how does it get updated after FSD?
nvm

gacholio · 2021-06-01T16:16:30Z

There is only ever one java.lang.Class instance - replacing classes results in multiple J9Class pointing to the single java.lang.Class, and the java object always points back to the current version of the J9Class.

Disable fixDirectHandles when building for OJDK method handles. Issue: eclipse-openj9#11528 Signed-off-by: Eric Yang <eric.yang@ibm.com>

babsingh · 2021-06-03T14:22:09Z

@EricYangIBM #12799 and #12806 were recently merged to resolve this issue. Can you evaluate if these PRs are sufficient to close this issue? If not, let us know the next steps.

EricYangIBM · 2021-06-03T14:24:22Z

@EricYangIBM #12799 and #12806 were recently merged to resolve this issue. Can you evaluate if these PRs are sufficient to close this issue? If not, let us know the next steps.

Yes I believe so.

babsingh · 2021-06-03T14:26:20Z

Closing issue as per #11528 (comment).

babsingh mentioned this issue Dec 21, 2020

Update linkToInterface for OpenJDK MethodHandles #11367

Merged

DanHeidinga added the project:MH Used to track Method Handles related work label Dec 22, 2020

DanHeidinga added this to the Release 0.25 (Java 16) milestone Dec 22, 2020

This was referenced Jan 12, 2021

Test Results for OpenJ9 JDK15/JDK16 with OpenJDK MethodHandles #11458

Closed

Transform OpenJDK MethodHandle methods #11634

Merged

tajila added comp:jit comp:vm labels Jan 13, 2021

tajila modified the milestones: Release 0.25 (Java 16), Release 0.26 (Java 8, 11, 16) Apr refresh Jan 19, 2021

tajila modified the milestones: Release 0.26 (Java 8, 11, 16) Apr refresh, Release 0.27 (Java 8, 11, 16) July refresh Mar 4, 2021

babsingh mentioned this issue Apr 1, 2021

OJDK MHs: Only append a non-null appendix to the stack #12306

Merged

babsingh mentioned this issue Apr 23, 2021

MethodHandle changes required for jdk17 #12524

Closed

babsingh assigned EricYangIBM May 17, 2021

EricYangIBM mentioned this issue May 31, 2021

Fix MemberName fields after Fast HCR and FSD #12799

Merged

EricYangIBM added a commit to EricYangIBM/openj9 that referenced this issue Jun 1, 2021

Disable J9-specific HCR code in OJDK implementation

7fae476

Disable fixDirectHandles when building for OJDK method handles. Issue: eclipse-openj9#11528 Signed-off-by: Eric Yang <eric.yang@ibm.com>

EricYangIBM mentioned this issue Jun 1, 2021

Disable J9-specific HCR code in OJDK implementation #12806

Merged

babsingh mentioned this issue Jun 3, 2021

OJDK MHs test excludes + Process a list of test exclude files #12779

Merged

babsingh closed this as completed Jun 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Account HCR for OpenJDK MethodHandles #11528

Account HCR for OpenJDK MethodHandles #11528

babsingh commented Dec 21, 2020

babsingh commented Dec 21, 2020

babsingh commented Dec 21, 2020

liqunl commented Jan 13, 2021

babsingh commented Jan 13, 2021

liqunl commented Jan 13, 2021

liqunl commented Jan 13, 2021

babsingh commented Jan 13, 2021

tajila commented Jan 13, 2021 •

edited

Loading

tajila commented Jan 13, 2021

babsingh commented Jan 13, 2021

gacholio commented Jan 13, 2021

babsingh commented Jan 13, 2021

tajila commented Jan 13, 2021

babsingh commented Jan 14, 2021 •

edited

Loading

babsingh commented Apr 1, 2021

EricYangIBM commented May 26, 2021

#11367 (comment) [Dan Heidinga]

DanHeidinga commented May 27, 2021

fengxue-IS commented May 27, 2021 •

edited

Loading

fengxue-IS commented May 27, 2021

DanHeidinga commented May 28, 2021

nbhuiyan commented May 28, 2021

fengxue-IS commented May 28, 2021 •

edited

Loading

gacholio commented May 31, 2021

EricYangIBM commented May 31, 2021 •

edited

Loading

gacholio commented May 31, 2021

EricYangIBM commented May 31, 2021

gacholio commented May 31, 2021

EricYangIBM commented Jun 1, 2021 •

edited

Loading

gacholio commented Jun 1, 2021

babsingh commented Jun 3, 2021

EricYangIBM commented Jun 3, 2021

babsingh commented Jun 3, 2021

Account HCR for OpenJDK MethodHandles #11528

Account HCR for OpenJDK MethodHandles #11528

Comments

babsingh commented Dec 21, 2020

#11367 (comment) [Babneet Singh]

#11367 (comment) [Dan Heidinga]

babsingh commented Dec 21, 2020

babsingh commented Dec 21, 2020

liqunl commented Jan 13, 2021

babsingh commented Jan 13, 2021

liqunl commented Jan 13, 2021

liqunl commented Jan 13, 2021

babsingh commented Jan 13, 2021

tajila commented Jan 13, 2021 • edited Loading

tajila commented Jan 13, 2021

babsingh commented Jan 13, 2021

gacholio commented Jan 13, 2021

babsingh commented Jan 13, 2021

tajila commented Jan 13, 2021

babsingh commented Jan 14, 2021 • edited Loading

babsingh commented Apr 1, 2021

EricYangIBM commented May 26, 2021

#11367 (comment) [Dan Heidinga]

DanHeidinga commented May 27, 2021

fengxue-IS commented May 27, 2021 • edited Loading

fengxue-IS commented May 27, 2021

DanHeidinga commented May 28, 2021

nbhuiyan commented May 28, 2021

fengxue-IS commented May 28, 2021 • edited Loading

gacholio commented May 31, 2021

EricYangIBM commented May 31, 2021 • edited Loading

gacholio commented May 31, 2021

EricYangIBM commented May 31, 2021

gacholio commented May 31, 2021

EricYangIBM commented Jun 1, 2021 • edited Loading

gacholio commented Jun 1, 2021

babsingh commented Jun 3, 2021

EricYangIBM commented Jun 3, 2021

babsingh commented Jun 3, 2021

tajila commented Jan 13, 2021 •

edited

Loading

babsingh commented Jan 14, 2021 •

edited

Loading

fengxue-IS commented May 27, 2021 •

edited

Loading

fengxue-IS commented May 28, 2021 •

edited

Loading

EricYangIBM commented May 31, 2021 •

edited

Loading

EricYangIBM commented Jun 1, 2021 •

edited

Loading