July 2019 - hotspot-gc-dev

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

[PATCH] Exploit Empty Regions in Young Gen to Enhance PS Full GC Performance
by Haoyu Li 18 Nov '19

18 Nov '19

Hi all， I have developed a patch to enhance the full GC performance of Parallel Scavenge on OpenJDK 11, may I have some reviews? The patch is described as follows and attached in this mail. *Problem* Parallel Scavenge(PS) implements a compacting algorithm to do the full GC, and we find that this algorithm leads in terrible GC thread utilization (like only 8% on Derby benchmark in SPECjvm2008 suite) since there are serious dependencies between heap regions, i.e., a region is available to receive live objects from its source regions only after it has been collected. The work stealing does not solve this problem, idle GC threads cannot steal anything because most regions are unavailable to collect. *Optimization* We propose *shadow region* to solve the above problem. The basic idea is to let GC threads collect unavailable regions in advance by copying their live data into newly allocated empty regions, i.e., shadow regions, to resolve the region dependencies. The contents of shadow regions will be copied back to the corresponding regions later. With our approach, GC threads can keep working at most of the time without suffering from any work stealing failure (except the work stealing failure happened in the end of a full GC). And we notice that the to space in young gen is always empty, so we exploit the empty regions in to space to play the role of shadow regoins (if the ScavengeBeforeFullGC option is on, regions in eden space may be used, too) and avoid allocating shadow regions from off heap memory. *Evaluation* We evaluate the full GC performance with our patch on DaCapo, SPECjvm2008, JOlden benchmark suits, and the results shows that shadow region optimization could improve full GC throughput by 2.1X on average, up to 3.2X. The patch and evaluation result are attached. Best Regrads, Haoyu Li, Institute of Parallel and Distributed Systems(IPADS), School of Software, Shanghai Jiao Tong University

5 31

RFR 8228532: Shenandoah: Implement SBSA::try_resolve_jobject_in_native()
by Zhengyu Gu 30 Oct '19

30 Oct '19

Shenandoah needs to take slow path when resolving jobject during evacuation phase, to ensure it resolves to-space oops. Bug: https://bugs.openjdk.java.net/browse/JDK-8228532 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8228532/webrev.00/index.html Test: hotspot_gc_shenandoah (fastdebug and release) on Linux x86_64 and aarch64. Thanks, -Zhengyu

4 11

G1 patch of elastic Java heap
by 毛亮(在弦) 13 Oct '19

13 Oct '19

Dear OpenJDK developers, Alibaba have developed an elastic heap feature for G1 GC and is willing to contribute it to OpenJDK. As we know that in cloud environment memory is the key resource and there is already a similar implemented proposal http://openjdk.java.net/jeps/346 in OpenJDK13, our patch is an independent implementation and contains some major enhancements as following: 1. stop-the-world free In current implementation heap shrink/expand happens in stop-the-world time (G1 remark or full GC) while uncommitting heap memory like several GB in Linux might cost hundreds of milliseconds and even up to seconds. Alibaba's online services cannot tolerate these long pauses so the new elastic heap feature do the memory uncommitment and commitment concurrently and asynchronously. The Java application can still run smoothly with G1 "low latency". 2. More effective memory return Current heap shrink heuristic relies on a major/full GC cycle(remark or full GC) to evaluate the heap usage while our patch's policy can treat young generation and old generation respectively. The Java application can promptly save the memory of young generation(default Max 60% of heap in G1) if young GC frequency is quite low and will soon recover the young generation's memory if young GC frequency boosts. The G1 elastic heap feature is already used in product environment in Alibaba cloud with AlibabaJDK8. We are glad to contribute it to OpenJDK upstream. Appreciate and look forward to your advises. Thanks, Liang

3 14

RFR: 8228369: Shenandoah: Refactor LRB C1 stubs
by Roman Kennke 12 Aug '19

12 Aug '19

Currently, interpreter and C1 are calling through a shared stub. This is somewhat rigid, and not really the C1-ish way to do it. In particular, it makes it very difficult to pass an additional argument to the runtime-call, which we're going to need for self-fixing-LRBs. This patch makes the C1 path go through its own stub that is now generated in the same way as other C1 runtime stubs. Notable changes: - the save_vector_registers()/restore_vector_registers() is no longer needed and we can instead re-use the similar methods from StubAssembler. This is also more useful maintenance-wise if the calling convention/registers ever change, if bugs come up in that code, etc. - The null-check in the CodeStub branches directly to the continuation instead of skipping the call. - Register allocation for the result forces rax/r0 (the return register), which means we can avoid some shuffling in the CodeStub. - Calling the runtime-stub now allows to pass parameters, which will be *very* helpful when we want to the sfx-lrbs. - In order to make the interpreter stub and the C1 stub share as much code as possible, I extracted gen_cset_check() and gen_resolved_check() to be used by both. We could in-fact consider to do away with the interpreter stub altogether, inline the whole code, and benefit from the passed-in tmp register, but I took the approach to not touch that code unless needed: it's quite a nightmare to get the register shuffling right with the interpreter (even more so than C1). Bug: https://bugs.openjdk.java.net/browse/JDK-8228369 Webrev: http://cr.openjdk.java.net/~rkennke/JDK-8228369/webrev.00/ Testing: hotspot_gc_shenandoah (x86_64, aarch64) Good? Roman

2 7

RFR 8215523: Request for clarification in unexpected OU decrease with CMS GC
by Poonam Parhar 12 Aug '19

12 Aug '19

Hello, Please review the changes for the fix of: Bug 8215523 <https://bugs.openjdk.java.net/browse/JDK-8215523>: jstat reports incorrect values for OU for CMS GC Webrev: http://cr.openjdk.java.net/~poonam/8215523/webrev.00/ Problem: With CMS, at certain points, 'OU' (Old usage) values reported in the jstat output increase first and then drop down. This is wrong because the 'old usage' drops down without any old generation collection. Cause: For the 'used' values of different memory spaces, jstat uses a periodic task to read these values and update the corresponding jstat counters. For CMS generation, there is a sampler that periodically updates the 'used' counter: class GenerationUsedHelper : public PerfLongSampleHelper { 103 private: 104 Generation* _gen; 105 106 public: 107 GenerationUsedHelper(Generation* g) : _gen(g) { } 108 109 inline jlong take_sample() { 110 return _gen->used(); 111 } 112 }; The above used() call in turn makes a call to CompactibleFreeListSpace::used(). 376 size_t CompactibleFreeListSpace::used() const { 377 return capacity() - free(); 378 } 379 380 size_t CompactibleFreeListSpace::free() const { 381 // "MT-safe, but not MT-precise"(TM), if you will: i.e. 382 // if you do this while the structures are in flux you 383 // may get an approximate answer only; for instance 384 // because there is concurrent allocation either 385 // directly by mutators or for promotion during a GC. 386 // It's "MT-safe", however, in the sense that you are guaranteed 387 // not to crash and burn, for instance, because of walking 388 // pointers that could disappear as you were walking them. 389 // The approximation is because the various components 390 // that are read below are not read atomically (and 391 // further the computation of totalSizeInIndexedFreeLists() 392 // is itself a non-atomic computation. The normal use of 393 // this is during a resize operation at the end of GC 394 // and at that time you are guaranteed to get the 395 // correct actual value. However, for instance, this is 396 // also read completely asynchronously by the "perf-sampler" 397 // that supports jvmstat, and you are apt to see the values 398 // flicker in such cases. 399 assert(_dictionary != NULL, "No _dictionary?"); 400 return (_dictionary->total_chunk_size(DEBUG_ONLY(freelistLock())) + 401 totalSizeInIndexedFreeLists() + 402 _smallLinearAllocBlock._word_size) * HeapWordSize; 403 } As stated above in the code comments, free() can return an imprecise approximate value when allocations are happening in the CompactibleFreeListSpace at the same time, e.g. during direct allocations or promotions. Solution: For CMS, maintain an internal variable for the space usage. This internal variable gets updated only at points where the CMS space usage computation is guaranteed to be accurate. jstat and other memory usage monitoring tools get access to this stable value of used space. Testing: jdk-tier1,jdk-tier2,jdk-tier3,hs-tier1,hs-tier2,hs-tier3 Thanks, Poonam

5 12

[8u] RFR: 8135318: CMS wrong max_eden_size for check_gc_overhead_limit
by Severin Gehwolf 09 Aug '19

09 Aug '19

Hi, Could I please get a review for this OpenJDK 8u backport? It's essentially the same patch as in JDK 9+, but it didn't apply cleanly since the surrounding code is different and the files were moved around. Adjustments I've done: JDK 8u uses a different file layout: src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp => src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp JDK 8u doesn't have JDK-8065972 and JDK-8065992, which account for the surrounding code changes. Note that JDK-8065992 makes the _young_gen instance variable ParNewGEneration* from Generation*. Bug: https://bugs.openjdk.java.net/browse/JDK-8135318 webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8135318/01/webrev/ JDK 9 change: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/dd0c55eac358 Testing: "tier1" testing of Linux x86_64 which showed no new regressions. Thoughts? Thanks, Severin

2 3

RFR: 8224659: Parallel GC: Use WorkGang (1: PCRefProcTask)
by Leo Korinth 09 Aug '19

09 Aug '19

Hi, Here is the first patch in a proposed patch series of eight that removes gcTaskManager and uses the WorkGang abstraction instead. This is primary a refactoring to cleanup code for future enhancements. After this change all GCs will use the WorkGang abstraction. # General Change 1-3 removes pcTask, 4-6 psTask, and then eventually, at nr 7, task manager is completely removed. Change 8 then both removes and obsoletes two product flags. The last enhancement has a CSR attached to it dealing with the flag removal. More than two thousands lines are removed, 4 JVM flags are removed or obsoleted. The old thread code is completely removed. In general I have tried to keep the old structure, and just done smaller improvements in code. Performance numbers seems to indicate that we go from roughly no improvements up to more than 40% improvements (!). Regressions seems rare. Performance improvements are primary in the young collections. In this first patch the WorkGang is added, and in the seventh patch, the gcTaskManager is removed. In between, both implementations will be used for different parts. In the end I will push everything together so that we will not see two sets of threads at the same time (although it does work). # This Patch This first patch does two things: 1) Adds work gang functionality + adds WorkGang named _workers to parallelScavengeHeap + sets active workers in psParallelCompact.cpp and psScavenge.cpp. The corresponding functionality in gcTaskManager is done by gc_task_manager()->set_active_gang() 2) Creates a PCRefProcTask that replaces RefProcTaskProxy and StealMarkingTask. The StealMarkingTask is removed in the next patch as it is still used by other parts. Enhancement: https://bugs.openjdk.java.net/browse/JDK-8224659 Webrev: http://cr.openjdk.java.net/~lkorinth/workgang/0/_8224659-Parallel-GC-Use-Wo… http://cr.openjdk.java.net/~lkorinth/workgang/0/all/ Testing (on the whole patch series): mach5 remote-build-and-test --build-profiles linux-x64,linux-x64-debug,macosx-x64,solaris-sparcv9,windows-x64 --test open/test/hotspot/jtreg/:hotspot_gc -a -XX:+UseParallelGC gc test suite Thanks, Leo

3 49

Re: RFR(XS) 8228359: [TESTBUG] jdk.jfr.e.g.c.TestGCHeapConfigurationEventWith32BitOops.java does not expect MinHeapSize to be aligned to HeapAlignment
by mikhailo.seledtsov＠oracle.com 07 Aug '19

07 Aug '19

Looks good from my POV. Adding hotspot-gc-dev(a)openjdk.java.net since this test concerns a GC event. Thank you, Misha On 7/18/19 6:56 AM, Reingruber, Richard wrote: > Hi, > > could I please get reviews for this small TESTBUG fix? > > Webrev: http://cr.openjdk.java.net/~rrich/webrevs/2019/8228359/webrev/ > Bug: https://bugs.openjdk.java.net/browse/JDK-8228359 > > Since JDK-8223837 MinHeapSize is aligned to HeapAlignment and the test fails therefore on linuxppc, because there we have 64k pages and HeapAlignment is proportional to the page size. > The fix changes the test to expect the configured size of 100m to be aligned to HeapAlignment (32m on linuxppc). > > Tested on linuxppcle and on linuxx86_64. > > Thanks, Richard.

2 1

RFR(S): 8227225: ParallelGC: add subspace transitions for young gen for gc+heap=info log lines
by Tony Printezis 06 Aug '19

06 Aug '19

Hi all, This is a follow-up to 8223575 and adds subspace info for eden and from space to the gc+heap=info output for ParallelGC. I also added the before capacity, similar to 8223575, as it can change during the GC (see, in fact, example below). Before: [11.123s][info][gc,heap ] GC(17) PSYoungGen: 4759616K->1664K(4545536K) [11.123s][info][gc,heap ] GC(17) ParOldGen: 1280K->1280K(1376256K) After: [12.133s][info][gc,heap ] GC(25) PSYoungGen: 2065952K(2066432K)->1536K(1983488K) Eden: 2064384K(2064384K)->0K(1981952K) From: 1568K(2048K)->1536K(1536K) [12.133s][info][gc,heap ] GC(25) ParOldGen: 1272K(1376256K)->1272K(1376256K) http://cr.openjdk.java.net/~tonyp/8227225/webrev.0/ I have a similar change ready to go for GenCollectedHeap (Serial / CMS). I’ll see what the feedback is on this before I open that for code review. A couple of notes: - I moved the PreGCValues class to gc/shared as I’d like to re-use it for the GenCollectedHeap changes too. - Is it worth also adding info about To space to the output? Tony PS Should I add the gc-pending-review label to any CR I open for code review? ————— Tony Printezis | @TonyPrintezis | tprintezis(a)twitter.com

3 15