Update resolve code to support OpenJDK MH #10893

fengxue-IS · 2020-10-14T22:21:29Z

Rename methodTypes table to invokeCache table
new sendResolveOpenJDKInvokeHandle
new resolveInvokeHandle
update resolveInvokeDynamic
update resolveVirtualMethodRefInto
update resolveStaticMethodRefInto
Modify Interpreter code to support the new single slot design
update old_slow_jitResolveHandleMethod

Related: #7352

Co-authored-by: Babneet Singh sbabneet@ca.ibm.com
Signed-off-by: Jack Lu Jack.S.Lu@ibm.com

DanHeidinga · 2020-10-15T14:59:46Z

@fengxue-IS can you @ me when this is ready to be reviewed?

DanHeidinga · 2020-10-30T13:37:46Z

@fengxue-IS is this ready for another review cycle?

fengxue-IS · 2020-11-04T18:34:49Z

@DanHeidinga Apology for the late reply, I have been trying to resolve a crash when testing this change locally enabling OpenJDK_MethodHandle flag. With Tobi & Babneet's help, the problem is because of java code called during memberName resolution and triggered GC which moved the heap object and need to be refetched.

This is ready for review, can you please take a look?

FYI @babsingh @liqunl @nbhuiyan

DanHeidinga · 2020-11-04T19:00:35Z

Thanks for the update Jack. I'm aiming to review this later this week

babsingh

A mixture of functional and formatting issues are identified.

runtime/bcutil/ConstantPoolMap.hpp

runtime/codert_vm/cnathelp.cpp

runtime/vm/BytecodeInterpreter.hpp

runtime/vm/resolvesupport.cpp

runtime/bcutil/ConstantPoolMap.hpp

runtime/codert_vm/cnathelp.cpp

DanHeidinga · 2020-11-06T16:27:54Z

runtime/gc_structs/ClassIterator.hpp

@@ -85,7 +85,11 @@ class GC_ClassIterator {
 		, _classStaticsIterator(env, clazz)
 		, _constantPoolObjectSlotIterator((J9JavaVM *)env->getLanguageVM(), clazz)
 		, _callSitesIterator(clazz)
+#if defined(J9VM_OPT_OPENJDK_METHODHANDLE)
+		, _methodTypesIterator(clazz->romClass->invokeCacheCount, clazz->invokeCache)


Shouldn't this iterator be renamed given it's 1) not iterating MTs anymore, 2) it's now dealing with the invokeCache?

@amicic opinions?

seems reasonable, but since I don't fully understand relationship between MH and invokeCache, I'll add @dmitripivkine

(from my narrow perspective) I'm wondering why the ifdef build flag has METHODHANDLE in the name, and should it be INVOKECACHE ?

The ifdef is for the other all feature which is using the OpenJDK MH implementation.

@fengxue-IS Can you open an issue to track renaming the GC_MethodTypesIterator to something like GC_InvokeCacheIterator? We need to track it as a cleanup task that should be done before the final cutover but doesn't need to be included in this PR

created #11168 to track this cleanup work

runtime/oti/j9nonbuilder.h

runtime/vm/resolvesupport.cpp

runtime/codert_vm/cnathelp.cpp

DanHeidinga · 2020-11-10T14:21:02Z

@fengxue-IS ping me when the review has been addressed and I'll give this another look

runtime/vm/resolvesupport.cpp

fengxue-IS · 2020-11-12T19:34:11Z

Fixed formating/comments as suggested in review
rename resolveInvokeHandle to resolveOpenJDKInvokeHandle for consistency
new Trc_VM_Assert_ShouldNeverHappen for unreachable code path
added writeBarrier before CAS on the result array to ensure array elements are written before the array can be accessed
modified method lookup to use J9_LOOK_DIRECT_NAS flag

@DanHeidinga thanks for the feedback, can you please take another look?

DanHeidinga · 2020-11-12T20:10:28Z

@liqunl are you OK with the JIT helper change?

DanHeidinga · 2020-11-12T20:10:50Z

Jenkins test sanity zlinux jdk8

DanHeidinga · 2020-11-12T20:10:59Z

Jenkins test sanity win jdk11

DanHeidinga · 2020-11-12T20:11:14Z

Jenkins test sanity osx jdk15

fengxue-IS · 2020-11-12T21:23:03Z

Fixed compiler warning about unused variable nullSignature, local linux Java 11 compile passed.
@DanHeidinga can you restart the pr build

DanHeidinga · 2020-11-12T21:56:32Z

Jenkins test sanity osx jdk15

DanHeidinga · 2020-11-12T21:56:41Z

Jenkins test sanity win jdk11

DanHeidinga · 2020-11-12T21:56:51Z

Jenkins test sanity zlinux jdk8

DanHeidinga · 2020-11-13T16:18:21Z

Note, all the builds have passed now. I'm ready to merge this once the conflict is resolved.

Any remaining concerns @babsingh @liqunl?

@fengxue-IS can you resolve the conflict in jvm.tdf?

babsingh · 2020-11-13T16:44:26Z

No more concerns.

Co-authored-by: Babneet Singh <sbabneet@ca.ibm.com> Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>

- new sendResolveOpenJDKInvokeHandle - new resolveOpenJDKInvokeHandle - update resolveInvokeDynamic - update resolveVirtualMethodRefInto - update resolveStaticMethodRefInto Co-authored-by: Babneet Singh <sbabneet@ca.ibm.com> Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>

Modify Interpreter code to support the new single slot design Co-authored-by: Babneet Singh <sbabneet@ca.ibm.com> Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>

- update old_slow_jitResolveHandleMethod Co-authored-by: Babneet Singh <sbabneet@ca.ibm.com> Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>

Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>

fengxue-IS · 2020-11-13T17:16:15Z

Fixed conflict in j9vm.tdf, local compile passed

fengxue-IS force-pushed the jsr292_resolve2 branch from 6c8ea48 to 8238f0f Compare October 14, 2020 22:25

DanHeidinga self-requested a review October 15, 2020 14:59

DanHeidinga added comp:vm project:MH Used to track Method Handles related work labels Oct 15, 2020

This was referenced Oct 15, 2020

Update resolvesupport.cpp to support OpenJDK MH #10762

Closed

Research replace MethodHandle class with OpenJDK implementation on JDK Next #7352

Closed

fengxue-IS force-pushed the jsr292_resolve2 branch from 8238f0f to 93078a5 Compare October 20, 2020 20:14

DanHeidinga self-assigned this Oct 30, 2020

fengxue-IS marked this pull request as ready for review November 4, 2020 18:34

fengxue-IS changed the title ~~[WIP] Update resolve code to support OpenJDK MH~~ Update resolve code to support OpenJDK MH Nov 4, 2020

babsingh requested changes Nov 4, 2020

View reviewed changes

fengxue-IS force-pushed the jsr292_resolve2 branch 2 times, most recently from b959b48 to 8e73fef Compare November 5, 2020 22:58

DanHeidinga reviewed Nov 6, 2020

View reviewed changes

liqunl reviewed Nov 7, 2020

View reviewed changes

runtime/codert_vm/cnathelp.cpp Show resolved Hide resolved

fengxue-IS force-pushed the jsr292_resolve2 branch from 8e73fef to 391f187 Compare November 12, 2020 16:55

DanHeidinga reviewed Nov 12, 2020

View reviewed changes

runtime/vm/resolvesupport.cpp Outdated Show resolved Hide resolved

DanHeidinga reviewed Nov 12, 2020

View reviewed changes

runtime/vm/resolvesupport.cpp Outdated Show resolved Hide resolved

DanHeidinga reviewed Nov 12, 2020

View reviewed changes

runtime/vm/resolvesupport.cpp Show resolved Hide resolved

fengxue-IS force-pushed the jsr292_resolve2 branch from 391f187 to a7cc768 Compare November 12, 2020 19:23

fengxue-IS mentioned this pull request Nov 12, 2020

Renaming the GC_MethodTypesIterator to GC_InvokeCacheIterator for OpenJDK MH (JSR292) #11168

Open

fengxue-IS force-pushed the jsr292_resolve2 branch 2 times, most recently from 955d40f to 2a3dd87 Compare November 12, 2020 21:05

fengxue-IS and others added 5 commits November 13, 2020 11:47

Rename methodTypes table to invokeCache table

2fb1873

Co-authored-by: Babneet Singh <sbabneet@ca.ibm.com> Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>

Update BytecodeInterpreter to support OpenJDK MH

5eca3dd

Modify Interpreter code to support the new single slot design Co-authored-by: Babneet Singh <sbabneet@ca.ibm.com> Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>

Update JIT helper to support OpenJDK MH

64534a7

- update old_slow_jitResolveHandleMethod Co-authored-by: Babneet Singh <sbabneet@ca.ibm.com> Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>

Add tracepoints for invokeHandle and invokeDynamic resolution

1a9fb65

Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>

fengxue-IS force-pushed the jsr292_resolve2 branch from 2a3dd87 to 1a9fb65 Compare November 13, 2020 16:51

DanHeidinga merged commit 246476c into eclipse-openj9:master Nov 13, 2020

babsingh mentioned this pull request Nov 18, 2020

Should MethodHandles be always enabled in OpenJ9? #11225

Closed

babsingh mentioned this pull request Nov 26, 2020

Support for OpenJDK MethodHandles #11281

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update resolve code to support OpenJDK MH #10893

Update resolve code to support OpenJDK MH #10893

fengxue-IS commented Oct 14, 2020 •

edited

Loading

DanHeidinga commented Oct 15, 2020

DanHeidinga commented Oct 30, 2020

fengxue-IS commented Nov 4, 2020

DanHeidinga commented Nov 4, 2020

babsingh left a comment •

edited

Loading

DanHeidinga Nov 6, 2020

amicic Nov 6, 2020

amicic Nov 6, 2020 •

edited

Loading

DanHeidinga Nov 10, 2020

DanHeidinga Nov 12, 2020

fengxue-IS Nov 12, 2020

DanHeidinga commented Nov 10, 2020

fengxue-IS commented Nov 12, 2020 •

edited

Loading

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

fengxue-IS commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 13, 2020

babsingh commented Nov 13, 2020

fengxue-IS commented Nov 13, 2020

Update resolve code to support OpenJDK MH #10893

Update resolve code to support OpenJDK MH #10893

Conversation

fengxue-IS commented Oct 14, 2020 • edited Loading

DanHeidinga commented Oct 15, 2020

DanHeidinga commented Oct 30, 2020

fengxue-IS commented Nov 4, 2020

DanHeidinga commented Nov 4, 2020

babsingh left a comment • edited Loading

Choose a reason for hiding this comment

DanHeidinga Nov 6, 2020

Choose a reason for hiding this comment

amicic Nov 6, 2020

Choose a reason for hiding this comment

amicic Nov 6, 2020 • edited Loading

Choose a reason for hiding this comment

DanHeidinga Nov 10, 2020

Choose a reason for hiding this comment

DanHeidinga Nov 12, 2020

Choose a reason for hiding this comment

fengxue-IS Nov 12, 2020

Choose a reason for hiding this comment

DanHeidinga commented Nov 10, 2020

fengxue-IS commented Nov 12, 2020 • edited Loading

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

fengxue-IS commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 12, 2020

DanHeidinga commented Nov 13, 2020

babsingh commented Nov 13, 2020

fengxue-IS commented Nov 13, 2020

fengxue-IS commented Oct 14, 2020 •

edited

Loading

babsingh left a comment •

edited

Loading

amicic Nov 6, 2020 •

edited

Loading

fengxue-IS commented Nov 12, 2020 •

edited

Loading