January 2020 - hotspot-gc-dev

Work-in-progress: 8236485: Epoch synchronization protocol for G1 concurrent refinement
by Man Cao 19 Apr '21

19 Apr '21

Hi all, I have written up a description and challenges for implementing an epoch synchronization protocol. This protocol is necessary for removing the StoreLoad fence in G1's post-write barrier (JDK-8226731) Description: https://bugs.openjdk.java.net/browse/JDK-8236485 Work-in-progress webrev: https://cr.openjdk.java.net/~manc/8236485/webrev_wip0/ There are two main challenges that I'm not sure how to resolve: - Triggering a thread-local handshake is a blocking operation that can pass a safepoint. - There are native post-write barriers executed by threads in native/VM state. Discussions and suggestions are highly appreciated! -Man

4 7

RFR: 8208636: Incresing bottom instead of decreasing top when an owner thread fails to take the last task
by Michihiro Horie 13 Apr '20

13 Apr '20

Dear all, Would you please review the following change? This is not a bug fix but refactoring. The taskqueue is implemented based on the ABP work-stealing algorithm as noted in comment. (It looks Chase-Lev work-stealing, which addressed the overflow problem of ABP work-stealing, but core algorithm is the same.) 231 // Arora, N. S., Blumofe, R. D., and Plaxton, C. G. 232 // Thread scheduling for multiprogrammed multiprocessors. 233 // Theory of Computing Systems 34, 2 (2001), 115-144. Correctness is presented with the following paper as also noted: 241 // Le, N. M., Pop, A., Cohen A., and Nardell, F. Z. 242 // Correct and efficient work-stealing for weak memory models 243 // Proceedings of the 18th ACM SIGPLAN symposium on Principles and 244 // practice of parallel programming (PPoPP 2013), 69-80 There is a slight difference between the current taskqueue and Chase-Lev's deque. When an owner thread and thieves try to take the last task in the deque and a thief succeeds in stealing the task, top is decreased by the owner thread in taskqueue, while bottom is increased in Chase-Lev's deque. Le et al. use the characteristics that top monotonically increases for the proofs in their paper. I think there is no reason to decrease top in taskqueue. One tricky thing in current taskqueue is that loading a task before the CAS trial in pop_global() avoids the side effect that owner thread can push (thus, overwrite with) a new task in the same slot at which a thief stole the last task. I think it is more straightforward that top is changeable only via CAS and monotonically increased. Releasing incremented bottom is not necessary because succeeding push will release the bottom. Please note there is no concern in concurrency because size() in pop_global () returns 0 for dirty_size()=N-1 and so there is no thief trying a steal. 180 return (sz == N - 1) ? 0 : sz; Bug: https://bugs.openjdk.java.net/browse/JDK-8208636 Webrev: http://cr.openjdk.java.net/~mhorie/8208636/webrev.00/ I ran JTREG and SPECjbb2015 to confirm there is no change in the test results and performance. Again this is just a refactoring and it's true current taskqueue works even if it is not aligned with the formal proofs partly. Best regards, -- Michihiro, IBM Research - Tokyo

2 2

RFR (S) 8232100: GC timings should use proper units for heap sizes
by Aleksey Shipilev 25 Mar '20

25 Mar '20

RFE: https://bugs.openjdk.java.net/browse/JDK-8232100 Webrev: https://cr.openjdk.java.net/~shade/8232100/webrev.01/ GC log prints heap sizes in selected GC events. Currently, it unconditionally uses "M" as the suffix for heap sizes, which makes GC logs too coarse on smaller heaps. This loses performance data accuracy, which is sometimes a dealbreaker in logs analysis. Let's make it into proper units. I ran many tests of my own, but would appreciate if somebody runs it through more comprehensive suite of tests, looking for tests that parse the GC logs for whatever reason. Testing: eyeballing GC logs, jdk-submit, hotspot_gc {g1, shenandoah, parallel} -- Thanks, -Aleksey

1 2

RFR (XS): 8234608: [TESTBUG] Memory leak in gc/g1/unloading/libdefine.cpp
by Man Cao 13 Feb '20

13 Feb '20

Hi all, Could anyone review this small patch contributed by my colleague Ian Rogers (irogers(a)google.com)? Bug: https://bugs.openjdk.java.net/browse/JDK-8234608 Webrev: https://cr.openjdk.java.net/~manc/8234608/webrev.00/ -Man

3 7

RFR: 8236073: G1: Use SoftMaxHeapSize to guide GC heuristics
by Liang Mao 11 Feb '20

11 Feb '20

Hi Thomas, I just uploaded the new patch for SoftMaxHeapSize. The shrink works immediately. The concurrent uncommit will be in a different patch. http://cr.openjdk.java.net/~luchsh/8236073.webrev.2/ Thanks, Liang ------------------------------------------------------------------ From:MAO, Liang <maoliang.ml(a)alibaba-inc.com> Send Time:2020 Jan. 16 (Thu.) 11:21 To:Thomas Schatzl <thomas.schatzl(a)oracle.com>; hotspot-gc-dev <hotspot-gc-dev(a)openjdk.java.net> Subject:Re: RFR: 8236073: G1: Use SoftMaxHeapSize to guide GC heuristics Hi Thomas, Yes. We can focus on the current concurrent shrinking for now. You are right that changing the default behavior will be sensitive since you need to cover all types of applications including throughput and low-latency while our previous patch is mostly designed for low-latency. We'll figure this out later:) Thanks, Liang ------------------------------------------------------------------ From:Thomas Schatzl <thomas.schatzl(a)oracle.com> Send Time:2020 Jan. 16 (Thu.) 01:57 To:"MAO, Liang" <maoliang.ml(a)alibaba-inc.com>; hotspot-gc-dev <hotspot-gc-dev(a)openjdk.java.net> Subject:Re: RFR: 8236073: G1: Use SoftMaxHeapSize to guide GC heuristics Hi, On Wed, 2020-01-15 at 20:53 +0800, Liang Mao wrote: > Hi Thomas, > > So G1 doesn't need to shrink below Xms if SoftMaxHeapSize is > below Xms, does it? > No, never shrink below MinHeapSize. > Another question is that no matter we have an additional option we > had better have 2 criterions. The first is for urgent expansion that > GCTimeRatio is quite low and concurrent expansion with frequent GCs > is more harmful and expansion should be done immediately. It's the > current default flow as we found that 12 is a good number below which > applications can obviously incur timeout errors. The second is to > keep the GCTimeRatio and memory footprint in a balanced state so > any adjustments are better to be concurrent. The original number 99 > fits well here. If we have only one option "GCTimeRatio", we might > not be able to achieve both. Maybe we can have a LowGCTimeRatio below > which suppose to be not acceptable and a HighTimeRatio which is > certainly healthy. So far the change has been about shrinking the heap concurrently, and not expansion. Let's concentrate on the issue at hand, i.e. see how heap shrinking at more places turns out. I believe there will be lots of tweaking needed for this change to not show too many regressions in other applications. Remember that the defaults should work well for a large body of applications, not just a few. There may be knobs to tune it for others. Then look concurrent expansion, at application phase changes in the application, how to detect, and how to react best. Just for reference, last time we changed the sizing algorithm it took a few months to get it "right", with mostly improvements all around. Thanks, Thomas

2 9

RFR (S): 8238160: Uniformize Parallel GC task queue variable names
by Thomas Schatzl 11 Feb '20

11 Feb '20

Hi all, can I have reviews for this small change that moves some global typedefs used only by Parallel GC from taskqueue.hpp to parallel gc files, and further makes naming of instances of these more uniform? CR: https://bugs.openjdk.java.net/browse/JDK-8238160 Webrev: http://cr.openjdk.java.net/~tschatzl/8238160/webrev/ Testing: local compilation Thanks, Thomas

3 3

RFR: 8237143: Eliminate DirtyCardQ_cbl_mon
by Kim Barrett 06 Feb '20

06 Feb '20

Please review this change to eliminate the DirtyCardQ_cbl_mon. This is one of the two remaining super-special "access" ranked mutexes. (The other is the Shared_DirtyCardQ_lock, whose elimination is covered by JDK-8221360.) There are three main parts to this change. (1) Replace the under-a-lock FIFO queue in G1DirtyCardQueueSet with a lock-free FIFO queue. (2) Replace the use of a HotSpot monitor for signaling activation of concurrent refinement threads with a semaphore-based solution. (3) Handle pausing of buffer refinement in the middle of a buffer in order to handle a pending safepoint request. This can no longer just push the partially processed buffer back onto the queue, due to ABA problems now that the buffer is lock-free. CR: https://bugs.openjdk.java.net/browse/JDK-8237143 Webrev: https://cr.openjdk.java.net/~kbarrett/8237143/open.00/ Testing: mach5 tier1-5 Normal performance testing showed no significant change. specjbb2015 on a very big machine showed a 3.5% average critical-jOPS improvement, though not statistically significant; removing contention for that lock by many hardware threads may be a little bit noticeable.

3 7

RFR (XS): 8238229: Remove TRACESPINNING debug code
by Thomas Schatzl 03 Feb '20

03 Feb '20

Hi all, can I have reviews for this removal of some debug code in the TaskTerminator class? The code counts the total number of yields/peeks/spins during task termination. Since it is guarded by a define, the code is not included in any regular build, so there is potential for bit-rotting. Since the code is not very complicated (and I believe it is too simple for real measurements), and needs rebuild anyway for use I propose to just remove it instead of trying to improve it for unknown requirements. Based on JDK-8238220. CR: https://bugs.openjdk.java.net/browse/JDK-8238229 Webrev: http://cr.openjdk.java.net/~tschatzl/8238229/webrev/ Testing: local compilation Thanks, Thomas

3 4

RFR (S): 8238220: Rename OWSTTaskTerminator to TaskTerminator
by Thomas Schatzl 03 Feb '20

03 Feb '20

Hi all, can I have reviews for this renaming change of OWSTTaskTerminator to TaskTerminator now that there is only one task termination protocol implementation? I believe that the OWST prefix only makes the code harder to read without conveying interesting information at the uses. Based on JDK-8215297. CR: https://bugs.openjdk.java.net/browse/JDK-8238220 Webrev: http://cr.openjdk.java.net/~tschatzl/8238220/webrev/ Testing: local compilation Thanks, Thomas

3 5

RFR(S): 8237776: Shenandoah: Wrong result with Lucene test
by Roland Westrelin 03 Feb '20

03 Feb '20

http://cr.openjdk.java.net/~roland/8237776/webrev.00/ xmm0 (an argument to a call) gets corrupted in the c2i adapter (when going from c1 code to the interpreter) at the ShenandoahRuntime::write_ref_field_pre_entry() runtime call. That call is in the c2i because of c2i_entry_barrier() and resolve_weak_handle(). The proposed fix saves all floating point argument registers. Roland.

4 9