RFR: 8204210: Implementation: JEP 333: ZGC: A Scalable Low-Latency Garbage Collector (Experimental)

Per Liden per.liden at oracle.com
Wed Jun 6 14:16:37 UTC 2018


Hi Roman,

On 06/06/2018 11:32 AM, Roman Kennke wrote:
> I'm looking mostly at shared code changes. I can't really say much about
> C2, JFR, SA, tests and ZGC itself.
> 
> Some comments/questions:
> 
> - src/hotspot/share/classfile/vmSymbols.cpp:
>   why are you enabling the clone intrinsic unconditionally and not under
> the usual:
> if (!InlineObjectCopy || !InlineArrayCopy) return true;
> ?

Please note that the function is called is_disabled_by_flags(). So we're 
actually disabling (not enabling) it unconditionally for ZGC.

> 
> - src/hotspot/share/oops/instanceRefKlass.inline.hpp
> I wonder if this makes sense to upstream separately? Also, I'm curious

Yes, I think you're right. I don't see a reason why this couldn't be 
upstreamed separately.

I filed https://bugs.openjdk.java.net/browse/JDK-8204474 and sent an RFR 
to hotspot-dev.

> why we need to distinguish between weak and phantom?

Generally speaking, we should always annotate oop accesses with the 
proper strengths, which is why we distinguish between weak and phantom 
here. For ZGC, in this very specific case, we will deep down in the 
barrier code eventually do the same thing in both cases. However, such 
decisions should be made by the GC/BarrierSet and not the access 
call-site. A different GC might want to make a different decision.

An alternative would be to use ON_UNKNOWN_OOP_REF, which tells the 
BarrierSet that is needs to apply additional logic to figure out what 
the strength really is. However, this comes with a run-time overhead, 
and since we already have the type in try_discover() we can use that to 
do the proper access and avoid that overhead.

> 
> - src/hotspot/share/runtime/stackValue.cpp
> There's no reasonable way to abstract this via GC interface?

This code is there to solve a very ZGC specific issue, which is that a 
deopt can happens between a load and its load barrier. As I mentioned in 
the initial RFR mail, this will go away in our next iteration of our C2 
load barriers [1], which will by design make sure that this can't 
happen. We therefore didn't think it was worth the effort to create a 
abstraction for this, since it's highly ZGC specific and such an 
abstraction would become useless/unused pretty soon anyway.

[1] Nils is currently working on this, but it will not be part of the 
initial upstreaming of ZGC.

> 
> Very nice work!

Thanks Roman! And thanks a lot for reviewing!

/Per

> 
> Thanks,
> Roman
> 
> 
>> Hi all,
>>
>> Here are updated webrevs reflecting the feedback received so far.
>>
>> ZGC Master
>>    Incremental:
>> http://cr.openjdk.java.net/~pliden/8204210/webrev.0vs1-master
>>    Full: http://cr.openjdk.java.net/~pliden/8204210/webrev.1-master
>>
>> ZGC Testing
>>    Incremental:
>> http://cr.openjdk.java.net/~pliden/8204210/webrev.0vs1-testing
>>    Full: http://cr.openjdk.java.net/~pliden/8204210/webrev.1-testing
>>
>> Thanks!
>>
>> /Per
>>
>> On 06/01/2018 11:41 PM, Per Liden wrote:
>>> Hi,
>>>
>>> Please review the implementation of JEP 333: ZGC: A Scalable
>>> Low-Latency Garbage Collector (Experimental)
>>>
>>> Please see the JEP for more information about the project. The JEP is
>>> currently in state "Proposed to Target" for JDK 11.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8197831
>>>
>>> Additional information in can also be found on the ZGC project wiki.
>>>
>>> https://wiki.openjdk.java.net/display/zgc/Main
>>>
>>>
>>> Webrevs
>>> -------
>>>
>>> To make this easier to review, we've divided the change into two webrevs.
>>>
>>> * ZGC Master: http://cr.openjdk.java.net/~pliden/8204210/webrev.0-master
>>>
>>>     This patch contains the actual ZGC implementation, the new unit
>>> tests and other changes needed in HotSpot.
>>>
>>> * ZGC Testing:
>>> http://cr.openjdk.java.net/~pliden/8204210/webrev.0-testing
>>>
>>>     This patch contains changes to existing tests needed by ZGC.
>>>
>>>
>>> Overview of Changes
>>> -------------------
>>>
>>> Below follows a list of the files we add/modify in the master patch,
>>> with a short summary describing each group.
>>>
>>> * Build support - Making ZGC an optional feature.
>>>
>>>     make/autoconf/hotspot.m4
>>>     make/hotspot/lib/JvmFeatures.gmk
>>>     src/hotspot/share/utilities/macros.hpp
>>>
>>> * C2 AD file - Additions needed to generate ZGC load barriers (adlc
>>> does not currently offer a way to easily break this out).
>>>
>>>     src/hotspot/cpu/x86/x86.ad
>>>     src/hotspot/cpu/x86/x86_64.ad
>>>
>>> * C2 - Things that can't be easily abstracted out into ZGC specific
>>> code, most of which is guarded behind a #if INCLUDE_ZGC and/or if
>>> (UseZGC) condition. There should only be two logic changes (one in
>>> idealKit.cpp and one in node.cpp) that are still active when ZGC is
>>> disabled. We believe these are low risk changes and should not
>>> introduce any real change i behavior when using other GCs.
>>>
>>>     src/hotspot/share/adlc/formssel.cpp
>>>     src/hotspot/share/opto/*
>>>     src/hotspot/share/compiler/compilerDirectives.hpp
>>>
>>> * General GC+Runtime - Registering ZGC as a collector.
>>>
>>>     src/hotspot/share/gc/shared/*
>>>     src/hotspot/share/runtime/vmStructs.cpp
>>>     src/hotspot/share/runtime/vm_operations.hpp
>>>     src/hotspot/share/prims/whitebox.cpp
>>>
>>> * GC thread local data - Increasing the size of data area by 32 bytes.
>>>
>>>     src/hotspot/share/gc/shared/gcThreadLocalData.hpp
>>>
>>> * ZGC - The collector itself.
>>>
>>>     src/hotspot/share/gc/z/*
>>>     src/hotspot/cpu/x86/gc/z/*
>>>     src/hotspot/os_cpu/linux_x86/gc/z/*
>>>     test/hotspot/gtest/gc/z/*
>>>
>>> * JFR - Adding new event types.
>>>
>>>     src/hotspot/share/jfr/*
>>>     src/jdk.jfr/share/conf/jfr/*
>>>
>>> * Logging - Adding new log tags.
>>>
>>>     src/hotspot/share/logging/*
>>>
>>> * Metaspace - Adding a friend declaration.
>>>
>>>     src/hotspot/share/memory/metaspace.hpp
>>>
>>> * InstanceRefKlass - Adjustments for concurrent reference processing.
>>>
>>>     src/hotspot/share/oops/instanceRefKlass.inline.hpp
>>>
>>> * vmSymbol - Disabled clone intrinsic for ZGC.
>>>
>>>     src/hotspot/share/classfile/vmSymbols.cpp
>>>
>>> * Oop Verification - In four cases we disabled oop verification
>>> because it do not makes sense or is not applicable to a GC using load
>>> barriers.
>>>
>>>     src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp
>>>     src/hotspot/cpu/x86/stubGenerator_x86_64.cpp
>>>     src/hotspot/share/compiler/oopMap.cpp
>>>     src/hotspot/share/runtime/jniHandles.cpp
>>>
>>> * StackValue - Apply a load barrier in case of OSR. This is a bit of a
>>> hack. However, this will go away in the future, when we have the next
>>> iteration of C2's load barriers in place (aka "C2 late barrier
>>> insertion").
>>>
>>>     src/hotspot/share/runtime/stackValue.cpp
>>>
>>> * JVMTI - Adding an assert() to catch problems if the tagmap hashing
>>> is changed in the future.
>>>
>>>     src/hotspot/share/prims/jvmtiTagMap.cpp
>>>
>>> * Legal - Adding copyright/license for 3rd party hash function used in
>>> ZHash.
>>>
>>>     src/java.base/share/legal/c-libutl.md
>>>
>>> * SA - Adding basic ZGC support.
>>>
>>>     src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/*
>>>
>>>
>>> Testing
>>> -------
>>>
>>> * Unit testing
>>>
>>>     A number of new ZGC specific gtests have been added, in
>>> test/hotspot/gtest/gc/z/
>>>
>>> * Regression testing
>>>
>>>     No new failures in Mach5, with ZGC enabled, tier{1,2,3,4,5,6}
>>>     No new failures in Mach5, with ZGC disabled, tier{1,2,3}
>>>
>>> * Stress testing
>>>
>>>     We have been continuously been running a number stress tests
>>> throughout the development, these include:
>>>
>>>       specjbb2000
>>>       specjbb2005
>>>       specjbb2015
>>>       specjvm98
>>>       specjvm2008
>>>       dacapo2009
>>>       test/hotspot/jtreg/gc/stress/gcold
>>>       test/hotspot/jtreg/gc/stress/systemgc
>>>       test/hotspot/jtreg/gc/stress/gclocker
>>>       test/hotspot/jtreg/gc/stress/gcbasher
>>>       test/hotspot/jtreg/gc/stress/finalizer
>>>       Kitchensink
>>>
>>>
>>> Thanks!
>>>
>>> /Per, Stefan & the ZGC team
> 
> 


More information about the hotspot-dev mailing list