RFR: 8204210: Implementation: JEP 333: ZGC: A Scalable Low-Latency Garbage Collector (Experimental)
Stefan Karlsson
stefan.karlsson at oracle.com
Thu Jun 7 05:30:13 UTC 2018
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