RFR: 8204210: Implementation: JEP 333: ZGC: A Scalable Low-Latency Garbage Collector (Experimental)
Per Liden
per.liden at oracle.com
Mon Jun 4 12:48:52 UTC 2018
Hi Stefan,
Thanks for reviewing. See comments inline below. I'm linking to fixes in
the zgc/zgc repo, and will let more comment come in before creating a
new webrev.
On 06/04/2018 11:35 AM, Stefan Johansson wrote:
> Hi guys,
>
> On 2018-06-01 23:41, 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.
>>
> I've looked at the shared GC parts and some other related code that I
> feel comfortable reviewing. The changes looks good in general, but a few
> comments:
>
> src/hotspot/share/gc/shared/specialized_oop_closures.hpp
> 39 #include "gc/z/zOopClosures.specialized.hpp"
>
> Any good reason for not following the naming convention used by all
> other collectors? Same goes for zFlags.hpp in gc_globals.hpp.
Fixed.
http://hg.openjdk.java.net/zgc/zgc/rev/db90a819d4ab
http://hg.openjdk.java.net/zgc/zgc/rev/e019298c2791
> ---
>
> src/hotspot/share/jfr/metadata/metadata.xml
> 897 <Event name="ZThreadPhase" category="Java Virtual Machine, GC,
> Detailed" label="ZGC Thread Phase" thread="true">
> 898 <Field type="uint" name="gcId" label="GC Identifier"
> relation="GcId"/>
> 899 <Field type="string" name="name" label="Name" />
> 900 </Event>
> 901
> 902 <Event name="ZStatisticsCounter" category="Java Virtual Machine,
> GC, Detailed" label="Z Statistics Counter" thread="true">
> 903 <Field type="ZStatisticsCounter" name="id" label="Id" />
> 904 <Field type="ulong" name="increment" label="Increment" />
> 905 <Field type="ulong" name="value" label="Value" />
> 906 </Event>
> 907
> 908 <Event name="ZStatisticsSampler" category="Java Virtual Machine,
> GC, Detailed" label="Z Statistics Sampler" thread="true">
> 909 <Field type="ZStatisticsSampler" name="id" label="Id" />
> 910 <Field type="ulong" name="value" label="Value" />
> 911 </Event>
> 912
> 913 <Type name="ZStatCounterType" label="Z Stat Counter">
> 914 <Field type="string" name="counter" label="Counter" />
> 915 </Type>
> 916
> 917 <Type name="ZStatSamplerType" label="Z Stat Sampler">
> 918 <Field type="string" name="sampler" label="Sampler" />
> 919 </Type>
>
> The fields in the events doesn't match the type definitions below,
> Statistics vs. Stat.
Fixed.
http://hg.openjdk.java.net/zgc/zgc/rev/01db0539fa1c
> ----
>
> src/hotspot/share/oops/instanceRefKlass.inline.hpp
> 61 if (type == REF_PHANTOM) {
> 62 referent = HeapAccess<ON_PHANTOM_OOP_REF |
> AS_NO_KEEPALIVE>::oop_load(java_lang_ref_Reference::referent_addr_raw(obj));
>
> 63 } else {
> 64 referent = HeapAccess<ON_WEAK_OOP_REF |
> AS_NO_KEEPALIVE>::oop_load(java_lang_ref_Reference::referent_addr_raw(obj));
>
> 65 }
>
> Extract this to a load_referent() helper function to help readability.
Fixed.
http://hg.openjdk.java.net/zgc/zgc/rev/d64cd7cb3d13
> ---
>
> src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp
> src/hotspot/share/runtime/jniHandles.cpp
> #if INCLUDE_ZGC
> // ...
> if (!UseZGC)
> #endif
>
> The UseZGC-flag is always available so the #if INCLUDE_ZGC can be
> skipped in the two places where this pattern is used.
Fixed.
http://hg.openjdk.java.net/zgc/zgc/rev/2cf588273130
Thanks!
/Per
> ---
>
> Cheers,
> Stefan
>
>
>> * 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