March 2019 - hotspot-gc-dev

RFR: 8219633: ZGC: Rename ZPageSizeMin to ZGranuleSize
by Per Liden 06 Mar '19

06 Mar '19

The size of the unit of memory managed by ZGC is currently called ZPageSizeMin. However, this constant is used in places where we're not necessarily dealing with a ZPage, so calling it ZPageSizeMin isn't quite right. Rename ZPageSizeMin to ZGranuleSize. Bug: https://bugs.openjdk.java.net/browse/JDK-8219633 Webrev: http://cr.openjdk.java.net/~pliden/8219633/webrev.0 /Per

3 6

RFR: 8219634: ZGC: Rename ZAddressRangeMap to ZGranuleMap
by Per Liden 06 Mar '19

06 Mar '19

The ZAddressRangeMap currently takes an AddressRangeShift template argument. However, that argument is always ZPageSizeMinShift (ZGranuleSizeShift after JDK-8219633). Simplify ZAddressRangeMap by removing the AddressRangeShift template argument and rename it ZGranuleMap to signal that it's always dealing with granules. Bug: https://bugs.openjdk.java.net/browse/JDK-8219634 Webrev: http://cr.openjdk.java.net/~pliden/8219634/webrev.0 /Per

3 6

RFR: 8219613: Use NonJavaThread PtrQueues
by Kim Barrett 05 Mar '19

05 Mar '19

Please review this change to G1 and Shenandoah to use the already extent thread-local PtrQueues in NonJavaThreads, rather than using shared queues that require locking. This is a step toward addressing JDK-8209974, JDK-8173211, and JDK-8170020. There are several parts to the change: The existing BarrierSet::on_thread_(attach|detach) functions get extended to apply to and be called for all threads, not just Java threads. A few implementations need to limit some of their behavior to Java threads, but others can just apply most or all of their existing behavior to non-Java threads too. This includes ZGC. Most operations involving shared queues are now applied to the corresponding thread-local queue, irrespective of whether the thread is a Java thread or not. Previously, a non-Java thread had to use the appropriate shared queue. Various iterations over threads get expanded to apply to all threads, rather than only Java threads. This change eliminates the single shared SATB queue and the associated (access-rank) lock. It does not eliminate the shared dirty card queues, as there is one use that can't be switched to using the thread-local queue. Because of that, the support for shared queues in PtrQueue (_lock and _permanent members and associated behavior) is still needed too. I have more changes in progress to address that. https://bugs.openjdk.java.net/browse/JDK-8219613 Webrev: http://cr.openjdk.java.net/~kbarrett/8219613/open.01/ gc/shared/barrierSet.cpp is in the webrev, but ended up not being changed. I'll update copyrights before pushing. Shenandoah changes provided with the help of Aleksey Shipilev. Testing: mach5 tier1-3, hs-tier4-5 jtreg: hotspot_gc_shenandoah

5 13

RFR(XXS) 8220161: Shenandoah does not need to initialize PLABs for safepoint workers
by zgu＠redhat.com 05 Mar '19

05 Mar '19

Please review this small patch that not creating PLAB for safepoint workers, it does not seem they evacuate anything. It also fixes the crashes we have seen recently, because we never enable PLAB creation for late safepoint workers, that results trying to retire NULL PLAB during GC. Bug: https://bugs.openjdk.java.net/browse/JDK-8220161 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8220161/webrev.00/ Test: tier1, 2, 3_gc_shenandoah on Linux 64 (fastdebug and release) Thanks, -Zhengyu

2 1

RFR (M): 8212206: Refactor AdaptiveSizePolicy to separate out code related to GC overhead
by Man Cao 05 Mar '19

05 Mar '19

Hi all, Could I have reviews for this refactoring change? Webrev: https://cr.openjdk.java.net/~manc/8212206/webrev.00/ RFE: https://bugs.openjdk.java.net/browse/JDK-8212206 It basically moves AdaptiveSizePolicy::check_gc_overhead_limit() to its own class so it can be shared with collectors that do not support AdaptiveSizePolicy. Each collector can provide its own functions to check if GC time or space overhead has exceeded threshold. It also removes one unused parameter "young_live". I think this is sufficient to share the common code for implementing UseGCOverheadLimit for G1 (JDK-8212084). Initially I tried moving all code for collecting GC time overhead in AdaptiveSizePolicy to a separate class, e.g. <minor|major>_collection_<begin|end>(). However, G1 already has its own ways to compute GC cost in G1Analytics. For example, G1HeapSizingPolicy::expansion_amount() already uses G1Analytics::_recent_avg_pause_time_ratio for GC cost. It is undesirable to have two ways to compute GC cost in one collector. In general, different collectors have different ways to compute GC time and space overhead. It could be too restrictive to make an interface to define how to collect GC time and space overhead. Thus, this patch only moves the code to check GC overhead in a common place. It could be reused to implement UseGCOverheadLimit for collectors such as ZGC in the future. There could be further refactoring to do on top of this patch. I'd like to get some opinions on whether they are desirable to do, and if they could be done as part of this RFE: (1) The develop flag AdaptiveSizePolicyGCTimeLimitThreshold could be renamed to GCOverheadLimitThreshold to be more accurate. I'm not sure about the process of renaming a develop flag, though. (2) The instance of the new class OverheadChecker could be a member of CollectedHeap, instead of a member of AdaptiveSizePolicy. Note that CollectedHeap::mem_allocate() already has a parameter "bool* gc_overhead_limit_was_exceeded". (3) The hsperf counter sun.gc.policy.gcTimeLimitExceeded is currently only available to ParallelGC. It can be moved to GCPolicyCounters so it is available to all collectors. Also gcTimeLimitExceeded is a misnomer, gcOverheadLimitExceeded is more accurate. Thanks, Man

4 16

[12u] RFR (XXS): 8218880: G1 crashes when issuing a periodic GC while the GCLocker is held
by Thomas Schatzl 05 Mar '19

05 Mar '19

Hi all, I would like to ask for reviews for the change for 8218880: G1 crashes when issuing a periodic GC while the GCLocker is held to backport to 12u. It does not apply cleanly due to some merge error in the copyright headers. CR: https://bugs.openjdk.java.net/browse/JDK-8218880 Webrev: http://hg.openjdk.java.net/jdk/jdk/rev/7935353a466a (jdk/jdk push) http://cr.openjdk.java.net/~tschatzl/8218880/webrev.12u.0_to_1/ (diff from jdk/jdk version) http://cr.openjdk.java.net/~tschatzl/8218880/webrev.12u/ (full) Testing: compilation; hs-tier1-5 almost completed thanks, Thomas

2 2

RFR(T) 8219976: GarbageCollectionNotificationInfo always says "No GC" when running Shenandoah
by zgu＠redhat.com 02 Mar '19

02 Mar '19

Please review this trivial fix, that sets heap's GC cause during GC session, so the value can be picked up, instead of default one. Bug: https://bugs.openjdk.java.net/browse/JDK-8219976 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8219976/webrev.00/ Test: tier1, 2, 3_gc_shenandoah on Linux 64 (fastdebug and release) Thanks, -Zhengyu

2 2

RFR (M): 8219747: Remove g1_ prefix to g1_remset and g1_policy members in G1CollectedHeap
by Thomas Schatzl 01 Mar '19

01 Mar '19

Hi all, can I have reviews for this renaming change that removes some "g1_" prefixes to some G1CollectedHeap members? They are simply unnecessary within the context of G1. There has also been a "g1_collector_policy" method - after looking at its uses I decided to remove it and add a specific whitebox support method for it to G1CollectedHeap instead. CR: https://bugs.openjdk.java.net/browse/JDK-8219747 Webrev: http://cr.openjdk.java.net/~tschatzl/8219747/webrev/ Testing: local compilation Thanks, Thomas

3 5

RFR (M): 8218880: G1 crashes when issuing a periodic GC while the GCLocker is held
by Thomas Schatzl 01 Mar '19

01 Mar '19

Hi all, can I have reviews for this change that fixes execution of periodic GCs in G1? In the current implementation, if periodic GC is triggered when the gc locker is held, the VM crashes because the GC locker is supposed to be only held by Java threads - but the periodic GCs are triggered by some GC thread. The fix is to ignore the periodic GC request if the GC locker is held - after all this means that the application is active anyway, and a GC will be triggered anyway as soon as the critical section ends. To make it clear to end users that the periodic GC has been cancelled due to that reason, the change prints a cancellation reason too if appropriate logging is enabled. The change itself is pretty small, it's just the test that's a bit longer but straightforward. I will backport this to 12u later. CR: https://bugs.openjdk.java.net/browse/JDK-8218880 Webrev: http://cr.openjdk.java.net/~tschatzl/8218880/webrev/ Testing: New test case, hs-tier1-5 Thanks, Thomas

3 8