Support non-tenured objects in the constantpool

[DanHeidinga]

JDK 11 introduced ConstantDynamic which allowed user allocated objects to be installed in classes ConstantPools (CP). This required changes to the GC so that the scavenges would scan the CP of classes with ConstantDynamics.

Our current JSR 292 implementation forces MethodType and MethodHandle objects that are stored into the CP to be tenured. Given we have the capability to scan CPs in scavenges, it would be good to remove the special case code to tenure MT & MH.

This also is an enabler for the investigation work in #7352 (Use OpenJDK LambdaForm MH implementation) as it makes it easier to consume the OpenJDK classes without patches.

Ideally, the existing store barriers would be sufficient to mark the classes as having CP entries that are in the young gen.

The existing special handling for Condy can be seen in https://github.com/eclipse/openj9/blob/master/runtime/gc_structs/ConstantPoolObjectSlotIterator.cpp

If we need to do special casing, then the following cases would need special handling:

• J9CPTYPE_METHOD_TYPE
• J9CPTYPE_METHODHANDLE
• J9CPTYPE_CONSTANT_DYNAMIC

Additionally, for classes loaded by Unsafe.defineAnonymousClass() and using constantpool patching, we would also need to scan J9CPTYPE_STRING as they can be used to "smuggle" other objects into the CP.

[DanHeidinga]

FYI @dmitripivkine @fengxue-IS

[dmitripivkine]

@LinHu2016 Would you please update code to scan all types of elements in CP in Scavenger for Java 11? Also would you please do general performance measure to know what cost of CP iteration is for Scavenger?

[LinHu2016]

@dmitripivkine Sure.

[LinHu2016]

I have done code update in GC, the performance comparison uses java 8 version(using Xgc option to enable/disable scanning CP object in scavenger) to run SPECjbb2015 preset compsite under regular gencon mode and concurrent scavenge mode.
the tests also disable numa and set fix tenure age(both default settings(numa and tenure age) could interfere the result, actually with enabling scan CP objects in scavenger would generate better performance).
result: extra CP iteration would generate 1% regression on scavenge pause for regular gencon mode and 2% regression for concurrent scavenge mode.
0.03-0.05% regression on total benchmark performance.

[DanHeidinga]

result: extra CP iteration would generate 1% regression on scavenge pause for regular gencon mode and 2% regression for concurrent scavenge mode.
0.03-0.05% regression on total benchmark performance.

@LinHu2016 Is that scanning all CPs for every class in every scavenge? Or only those that have a young-gen object in the CP?

[LinHu2016]

result: extra CP iteration would generate 1% regression on scavenge pause for regular gencon mode and 2% regression for concurrent scavenge mode.
0.03-0.05% regression on total benchmark performance.

@LinHu2016 Is that scanning all CPs for every class in every scavenge? Or only those that have a young-gen object in the CP?

@DanHeidinga we iterate all CPs for every class in every scavenge, but only nursery object in the CP might be copied. for test case there is no cp object in nursery.

[dmitripivkine]

@vijaysun-omr @amicic @DanHeidinga

Currently Scavenger does not scan Classes related metadata (except Class Statics). It is possible because all other class related objects must be allocated in Tenure mandatory.

However we can not keep this behaviour any more. Condy code require to scan Class Constant Pool (already implemented for Java 11). Also VM developers requested Class Constant Pool scan for MH improvements (for Java 8 and Java 11 both). An enabling of scan of full Classes related metadata caused a regression in Scavenge time "2% regression on scavenge pause for gencon mode, 6.25% regression for concurrent scavenge" #7636 (comment)
Please note that this change allows to remove mandatory tenuring for Classes related objects which return back some performance in allocation path as far as TLH allocation can be used now.

So I am proposing:

• merge Enable scan all slots in a class during Scavenge #7636 to allow VM Team add required improvements
• start work on optimization of scanning process by maintaining "Class needs to be scanned in Scavenger" flag (set when class-related object is allocated in Nursery (barrier style to prevent race conditions), clean up in Scavenger if there is no Nursery objects referenced from Class metadata any more). Considering the fact that Class related objects in Nursery would be tenured soon this optimization can be effective. An implementation of such optimization would require some development work obviously.

May I have your opinions please?

[DanHeidinga]

The 2% regression to scavenger pauses on gencon doesn't really concern me given this would be hard to notice in the existing pause times. The 6.25% for concurrent scavenger is more of an issue as concurrent scavenger is aimed at low pause scenarios.

I'm on board with this plan provided that we start work on the "Class needs to be scanned in Scavenger" flag immediately.

[vijaysun-omr]

Are the slowdowns in scavenge time reported in #7636 (comment) expected to be the norm ? Or is it the results from one benchmark that was tried ?

Otherwise I agree with the plan mentioned by @dmitripivkine and echo Dan's comment that the CS overhead is the more worrisome one.

[dmitripivkine]

Are the slowdowns in scavenge time reported in #7636 (comment) expected to be the norm ? Or is it the results from one benchmark that was tried ?

This is result for SpecJBB 2015 (which is shows average behaviour usually). It is possible to construct test case where impact would be more significant obviously. However as far as we need this functionality so let focus on improvement part.

@DanHeidinga @vijaysun-omr Thank you for comments
@LinHu2016 Would you please start work on optimization as soon as #7636 is merged?

[LinHu2016]

@dmitripivkine Sure.

[amicic]

The optimization is already there. In Scavenger, we historically scanned only static slots only for classes that had statics to Nursery. This was accomplished by remembering such classes through static barrier (J9WriteBarrierJ9ClassStore or appropriate jit wrapper) call after the store itself.

[The underlying mechanism was using the class object (that is known to be pre-tenured) to remember/add to the same RememberSet used for standard object Tenure->Nursery references]

#7636 will widen the scope of what we scan within a class, if a class is to be scanned. But decision logic whether to scan a class (based on whether it's in RS) is already there and unaffected.

We, of course, have to make sure that all other class slot stores indeed use the WB API after the store. (I'm only seeing a couple of sites that use it!?)

[DanHeidinga]

All writes to the constantpool should be occurring in resolvesupport.cpp using the J9STATIC_OBJECT_STORE macro to store objects.

Taking a quick look at it shows all the places I would expect are using the J9STATIC_OBJECT_STORE macro. Is there something else that's missing?

[amicic]

J9STATIC_OBJECT_STORE will do it (it will do both the store and than call the barrier above) and actually preferred (to me) from separating the store and barrier calls. But, all recently added slots (MH, MT, CD...) should comply, too.

[DanHeidinga]

J9Class->callSites is updated using the CAS barrier: https://github.com/eclipse/openj9/blob/66556b4a095d62b0a37915c1793a5e697f6237e8/runtime/vm/resolvesupport.cpp#L2139

J9Class->MethodTypes as used for MH.{invoke,invokeExact}:
https://github.com/eclipse/openj9/blob/66556b4a095d62b0a37915c1793a5e697f6237e8/runtime/vm/resolvesupport.cpp#L1427

J9Class->varHandleMethodTypes as used for VarHandle invokes:
https://github.com/eclipse/openj9/blob/66556b4a095d62b0a37915c1793a5e697f6237e8/runtime/vm/resolvesupport.cpp#L1598

ConstantDynamic updates to the CP:
https://github.com/eclipse/openj9/blob/66556b4a095d62b0a37915c1793a5e697f6237e8/runtime/vm/resolvesupport.cpp#L2081-L2082

And the "regular" CP updates use J9STATIC_OBJECT_STORE when storing objects.

Anything I've missed here?

[amicic]

All those look ok. In a few min manually scanning the class code I could not find any missed.

Btw, what about Java_java_lang_invoke_MethodType_makeTenured? I don't see it is used (return value ever stored anywhere).

And while on the topic of barriers, we need to track reads as well (which I had mentioned before). I believe most are missed like:
https://github.com/eclipse/openj9/blob/66556b4a095d62b0a37915c1793a5e697f6237e8/runtime/vm/resolvesupport.cpp#L1979
https://github.com/eclipse/openj9/blob/66556b4a095d62b0a37915c1793a5e697f6237e8/runtime/vm/resolvesupport.cpp#L1984
https://github.com/eclipse/openj9/blob/66556b4a095d62b0a37915c1793a5e697f6237e8/runtime/vm/resolvesupport.cpp#L2008

There are the key for enabling concurrent class scanning in CS.

[DanHeidinga]

Btw, what about Java_java_lang_invoke_MethodType_makeTenured? I don't see it is used (return value ever stored anywhere).

Interning of MethodTypes is handled in Java but only tenured objects can be interned in that table as they end up in the constantpool. The makeTenured() calls are only done when interning:
https://github.com/eclipse/openj9/blob/28944136f2e38e77d7c37d004e12fdfa4fcfc49d/jcl/src/java.base/share/classes/java/lang/invoke/MethodType.java#L755

All of this code will go away once the CPs are scanned during all scavenges.

And while on the topic of barriers, we need to track reads as well (which I had mentioned before).

That's going to be a major change as the BytecodeInterpreter and other code (likely lots of it) reads CP entries without read barriers today.

There are the key for enabling concurrent class scanning in CS.

Can the "Support non-tenured objects in the constantpool" be completed without enabling concurrent class scanning in CS? Or do the two have to land at the same time?

[amicic]

Enabling concurrent class scanning in CS is not strictly a pre-req for this work. It's an independent work item/optimization that will compensate the increase of class scanning time introduced by this work.

At some point (when only statics where part of Scavenge class scanning) we were very close to enabling it, but then came CP scanning which complied only with static stores barriers but not with static reads (which as I said are pre-req for enabling concurrent class scanning). And now, we are adding even more to class scanning (that also does not comply with read barriers) and making this optimization even harder to happen.

Anyhow, I just want to raise awareness of it, rather than holding completion of this or other work.

[DanHeidinga]

I opened #7718 to record the CS work that needs to be done in the future.