RFR: 8204210: Implementation: JEP 333: ZGC: A Scalable Low-Latency Garbage Collector (Experimental)
Stefan Karlsson
stefan.karlsson at oracle.com
Thu Jun 7 06:58:50 UTC 2018
On 2018-06-07 08:01, Jini George wrote:
> Thank you for making the changes, Stefan. One minor nit:
>
> * in HSDB.java, I meant that
>
> "ZHeap";
>
> needs to be changed to
>
> "ZHeap ";
Fixed.
>
> Sorry for not making it clear! Everything else looks good. I don't need
> to see another webrev.
Thanks,
StefanK
>
> 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