March 2019 - hotspot-gc-dev

RFR: 8221392: Reduce ConcurrentGCThreads spinning during start up
by Per Liden 27 Mar '19

27 Mar '19

During startup, ConcurrentGCThreads end up in a loop sleeping for 1ms over and over (typically 30-50 times), until is_init_completed() is true. This is not optimal, and we should instead have them wait until they are signaled. This problem has been observed before, as comments in JDK-8136854 suggests. This patch introduces wait_init_completed(), which will block until set_init_completed() is called. Note that the newly introduced InitCompleted_lock is only used to control the interaction between wait_init_completed() and set_init_completed(). is_init_completed() is still lock-free and was just cleaned up to use proper atomic load_acquire/release_store semantics. Bug: https://bugs.openjdk.java.net/browse/JDK-8221392 Webrev: http://cr.openjdk.java.net/~pliden/8221392/webrev.0 /Per

4 11

RFR 8221435: Shenandoah should not mark through weak roots
by Zhengyu Gu 26 Mar '19

26 Mar '19

Please review this patch that fixes incorrectly marking through weak roots in Shenandoah. Bug: https://bugs.openjdk.java.net/browse/JDK-8221435 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8221435/webrev.00/index.html Test: hotspot_gc_shenandoah on Linux x64 (fastdebug and release) Thanks, -Zhengyu

3 6

RFR (L): 8219100: Improve do_collection_pause_at_safepoint
by Thomas Schatzl 26 Mar '19

26 Mar '19

Hi all, can I have reviews for this (relatively big) cleanup change that reorganizes the G1CollectedHeap::do_collection_pause_at_safepoint() method to be (imho) much more readable? - refactored out some methods, mainly verification but also other helpers - grouped together "same" category calls so that e.g. collection set related methods are not more or less randomly interspersed with unrelated code - moved code into the right "levels" of indentation - moved code related to evacuation that kind of bleeded out of pre/post_evacuate_collection_set() into it, also trying to improve grouping and ordering CR: https://bugs.openjdk.java.net/browse/JDK-8219100 Webrev: http://cr.openjdk.java.net/~tschatzl/8219100/webrev Testing: hs-tier1-8 Thanks, Thomas

3 4

[URGENT] RFR (XS) 8221363: Build failure after JDK-8220240 (Refactor shared dirty card queue)
by Aleksey Shipilev 23 Mar '19

23 Mar '19

Bug: https://bugs.openjdk.java.net/browse/JDK-8221363 Super-class SATBMarkQueue dropped this: -SATBMarkQueue::SATBMarkQueue(SATBMarkQueueSet* qset, bool permanent) : +SATBMarkQueue::SATBMarkQueue(SATBMarkQueueSet* qset) : So the build fix is: diff -r c81fbf340ceb src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.hpp @@ -29,11 +29,11 @@ #include "runtime/mutex.hpp" #include "runtime/thread.hpp" class ShenandoahSATBMarkQueue: public SATBMarkQueue { public: - ShenandoahSATBMarkQueue(SATBMarkQueueSet* qset) : SATBMarkQueue(qset, /* permanent = */ false) {} + ShenandoahSATBMarkQueue(SATBMarkQueueSet* qset) : SATBMarkQueue(qset) {} virtual bool should_enqueue_buffer(); }; class ShenandoahSATBMarkQueueSet : public SATBMarkQueueSet { private: Testing: local Linux x86_64 build -Aleksey

3 3

RFR: 8220240: Refactor shared dirty card queue
by Kim Barrett 22 Mar '19

22 Mar '19

Please review this change which adds a new class, G1SharedDirtyCardQueue, in order to allow some simplifications of the PtrQueue class and classes derived from it. An instance of this new class has been added to the G1BarrierSet, and is now used by the one place that was formerly enqueuing to the shared dirty card queue in the barrier set's dirty card qset. As there are no longer any enqueues to the queue in a dirty card qset, that queue has been removed. (JDK-8219613 already removed the similar queue from SATB qsets). This also allows the elimination of the _lock and _permanent members of PtrQueue and associated code. Some places that were dealing with cards logged in the queue embedded in qsets (specifically abandon_logs and concatenate_logs) now deal with the new shared queue in the barrier set. This makes it even more obvious that these operations are incorrectly placed. They aren't really generic operations on dirty card qsets at all, and really belong to the barrier set and should be applied only to its qset. I intend to move them in a following cleanup. With the various changes to PtrQueue, PtrQueueSet, and their derivatives, here and in previous changes, I think we're close to (but not quite there yet) eliminating the need for two-phase construction of PtrQueueSets. I intend to look at the elimination of the initialize() function as a followup cleanup. G1SharedDirtyCardQueue is still lock-based, using Shared_DirtyCardQ_lock, which still needs to have access-related rank. It could be made lock-free, if the elimination of that access-related lock seems sufficiently important to warrant the extra complexity. The situation where the shared queue is needed is extremely rare, so contention for the lock isn't an issue. This can be addressed in a followup improvement. CR: https://bugs.openjdk.java.net/browse/JDK-8220240 Webrev: http://cr.openjdk.java.net/~kbarrett/8220240/open.00/ I'll update copyrights before pushing. Testing: mach5 tier1-3, hs-tier4-5

3 5

RFR: 8221260: Initialize more class members on construction, remove some unused ones
by Leo Korinth 22 Mar '19

22 Mar '19

Hi, This is a code cleanup. This change removes some warnings when opening the files from within Eclipse. I try to: - Initialize more class members as soon as possible - Remove some (5) unused members, and one unused faulty constructor - Prefer to use initializer list instead of constructor body Enhancement: https://bugs.openjdk.java.net/browse/JDK-8221260 Webrev: http://cr.openjdk.java.net/~lkorinth/8221260/00 Testing: Mach5 tier1-3 Thanks, Leo

3 3

RFR: 8221212: ZGC: Command-line flags should be marked experimental
by Per Liden 22 Mar '19

22 Mar '19

ZGC (the -XX:+UseZGC command-line flag) is marked experimental, which means that you always need to use -XX:+UnlockExperimentalVMOptions to unlock that option. However, some of ZGC's flags are incorrectly marked as product flags. This can be a bit confusing, since those flags (but not the -XX:+UseZGC flag) will show up when using -XX:PrintFlagsFinal, even though they are effectively experimental and has no use/meaning unless ZGC is enabled. We should not mark any ZGC flags as product flags, until the -XX:+UseZGC flag is made a product flag. I have not filed a CSR for this, since these flags are effectively experimental already today, but just aren't properly marked as such. Bug: https://bugs.openjdk.java.net/browse/JDK-8221212 Webrev: http://cr.openjdk.java.net/~pliden/8221212/webrev.0 /Per

3 4

RFR: 8221219: ZGC: Remove ZStallOnOutOfMemory option
by Per Liden 22 Mar '19

22 Mar '19

The ZStallOnOutOfMemory option was added in ZGC's early days, and was at that time useful when testing and debugging. These days it's not so useful, and I can't really see anyone wanting to use it when running a real application as is can/will cause pre-mature OOMEs to be thrown. I suggest we remove it. Bug: https://bugs.openjdk.java.net/browse/JDK-8221219 Webrev: http://cr.openjdk.java.net/~pliden/8221219/webrev.0 (Change builds on JDK-8221212, which is out for review) /Per

3 5

RFR: JDK-8221278: Shenandoah should not enqueue string dedup candidates during root scan
by Roman Kennke 21 Mar '19

21 Mar '19

In Shenandoah, during the init-mark phase, we are scanning the StringDedup roots (queues and tables) when string deduplication is enabled. For this, we're using a closure that also enqueues dedup canidates. Which means that we may end up concurrently reading AND writing the queues, which leads to corruption. This seems to make the TestStringDedupStress occasionally crash. Bug: https://bugs.openjdk.java.net/browse/JDK-8221278 Webrev: http://cr.openjdk.java.net/~rkennke/JDK-8221278/webrev.00/ Proposed change is simple: Simply remove the ENQUEUE_DEDUP block and related template fluff from init-mark. It means that strings that are (only) reachable via local variables and such will not be string-dedup-candidates. So what? ;-) Testing: hotspot_gc_shenandoah fine. Failing test, amplified to fail more reliably, run in loop 30x, all good. Ok? Roman

2 1

Does G1's C1 barrier implementation always call the runtime write barrier function?
by William Kemper 21 Mar '19

21 Mar '19

I've been comparing the JIT assembly emitted by C1 and C2 and have come to believe that the code emitted by C1 always calls the runtime stub for the write barrier. Is there a reason for this? It seems the interpreter's assembler and C2 both emit assembly instructions for the write barrier, but C1 doesn't? have I missed something? I'm looking at OpenJDK 11.0.2. Thanks, William

2 1