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

Jini George jini.george at oracle.com
Thu Jun 7 06:01:22 UTC 2018 

Thank you for making the changes, Stefan. One minor nit:

* in HSDB.java, I meant that

"ZHeap";

needs to be changed to

"ZHeap ";

Sorry for not making it clear! Everything else looks good. I don't need 
to see another webrev.

Thanks,
Jini.


On 6/7/2018 11:00 AM, Stefan Karlsson wrote:
> Hi Jini,
> 
> Thanks for reviewing this!
> 
> Here are fixes to the nits and copyright years:
>    http://cr.openjdk.java.net/~stefank/8204210/webrev.sa.02.delta/
>    http://cr.openjdk.java.net/~stefank/8204210/webrev.sa.01/
> 
> StefanK
> 
> On 2018-06-07 06:57, Jini George wrote:
>> Hi Stefan,
>>
>> The changes look good overall. Please update the copyright year for 
>> the changed files, where applicable. Some minor nits:
>>
>> * GCCause.java:
>>
>> There is an extra space before the newly added _z* enum values.
>>
>> * HSDB.java:
>>
>> Pls add a space after ZHeap in:
>>
>> anno ="ZHeap";
>>
>> Thanks!
>> Jini
>>
>>
>> On 6/5/2018 8:44 PM, Stefan Karlsson wrote:
>>> Hi Jini,
>>>
>>> For this version experimental version of ZGC we only have basic SA 
>>> support, so the collectLiveRegions feature is not implemented.
>>>
>>> Comments below:
>>>
>>> On 2018-06-05 14:50, Jini George wrote:
>>>> Hi Per,
>>>>
>>>> I have looked at only the SA portion. Some comments on that:
>>>>
>>>> ==>  share/classes/sun/jvm/hotspot/oops/ObjectHeap.java
>>>>
>>>> The method collectLiveRegions() would need to include code to 
>>>> iterate through the Zpages, and collect the live regions.
>>>>
>>>> ==> share/classes/sun/jvm/hotspot/HSDB.java
>>>>
>>>> The addAnnotation() method needs to handle the case of collHeap 
>>>> being an instance of ZCollectedHeap to avoid "Unknown generation" 
>>>> being displayed while displaying the Stack Memory for a mutator thread.
>>>
>>> Fixed.
>>>
>>>>
>>>> ==> share/classes/sun/jvm/hotspot/gc/shared/GCCause.java
>>>>
>>>> To the GCCause enum, it would be good to add the equivalents of the 
>>>> following GC causes. (though at this point, GCCause seems unused 
>>>> within SA).
>>>>
>>>>      _z_timer,
>>>>      _z_warmup,
>>>>      _z_allocation_rate,
>>>>      _z_allocation_stall,
>>>>      _z_proactive,
>>>
>>> Fixed.
>>>
>>>>
>>>> ==> share/classes/sun/jvm/hotspot/gc/shared/GCName.java
>>>>
>>>> Similarly, it would be good to add the equivalent of 'Z' in the 
>>>> GCName enum.
>>>
>>> Fixed.
>>>
>>>>
>>>> ==> share/classes/sun/jvm/hotspot/runtime/VMOps.java
>>>>
>>>> Again, it would be good to add 'ZOperation' to the VMOps enum 
>>>> (though it looks like it is already not in sync).
>>>
>>> Fixed.
>>>
>>>>
>>>> ==> share/classes/sun/jvm/hotspot/tools/HeapSummary.java
>>>>
>>>> The run() method would need to handle the ZGC case too to avoid the 
>>>> unknown CollectedHeap type exception with jhsdb jmap -heap:
>>>>
>>>> Also, the printGCAlgorithm() method would need to be updated to read 
>>>> in the UseZGC flag to avoid the default "Mark Sweep Compact GC" 
>>>> being displayed with jhsdb jmap -heap.
>>>
>>> Fixed.
>>>
>>>>
>>>> ==> share/classes/sun/jvm/hotspot/gc/z/ZHeap.java
>>>>
>>>> It would be great if printOn() (for the clhsdb command 'universe') 
>>>> would print the address range of the java heap as we have in other 
>>>> GCs (with ZAddressSpaceStart and ZAddressSpaceEnd?)
>>>
>>> ZGC uses three fixed 4 TB reserved memory ranges (on Linux x64). I 
>>> don't think it's as important to print these ranges as it is for the 
>>> other GCs.
>>>
>>>>
>>>> ==> test/hotspot/jtreg/serviceability/sa/TestUniverse.java
>>>> Please modify the above test to include zgc or include a separate SA 
>>>> test to test the universe output for zgc.
>>>
>>> Fixed.
>>>
>>> Here's a quick webrev of your suggested changes:
>>> http://cr.openjdk.java.net/~stefank/8204210/webrev.sa.01/
>>>
>>> Thanks,
>>> StefanK
>>>
>>>>
>>>> Thank you,
>>>> Jini.
>>>>
>>>>
>>>> On 6/2/2018 3:11 AM, 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