April 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(M): 8220089: G1 wastes a significant amount of space in survivor regions
by sangheon.kim＠oracle.com 21 May '19

21 May '19

Hi all, Can I have some reviews for the patch that changes to reuse survivor region as mutator region? Currently survivor region is released at the end of gc and then added to the next collection set. This means there would be waste if the survivor region has enough free space. As the survivor regions are converted to eden at the start of gc, reusing the *last* survivor region as a mutator region would maximize the use of heap region. This patch suggests to reuse the survivor region as described if the remaining free space satisfies a minimum threshold(currently, larger than or equal to minimum TLAB). Some benchmarks(specjbb2015, specjbb2005, specjvm2008) that I ran didn't have noticeable improvement. However, I think this change would benefit more with NUMA implementation because the NUMA implementation manages survivor region per NUMA node. i.e. more wastes on multiple node system. CR: https://bugs.openjdk.java.net/browse/JDK-8220089 webrev: http://cr.openjdk.java.net/~sangheki/8220089/webrev.0 (based on JDK-8218668 webrev.1 which is under review now) Testing: hs-tier1 ~ 5 Thanks, Sangheon

3 6

RFR (S/M): 8222492: G1 unnecessarily scans remembered set cards for regions that already have been evacuated
by Thomas Schatzl 17 May '19

17 May '19

Hi all, can I have reviews for this change that: - improves performance during and after incremental mixed gc (avoiding evacuation of dead objects, taking unnecessary space on the heap) - and avoids potential crashes due to the risk of evacuation failures in regions already evacuated? The problem are remembered set entries (cards, or from the optional collection set) from already evacuated regions: if they are blindly scanned, they may resurrect dead objects as they may contain actually dead references into the current collection set. This useless evacuation not only takes space, but may in turn cause evacuation failures in regions that have already been evacuated, and since with JDK-8218668 we only do evacuation failure handling in regions of the current increment, cause us to not properly reset HeapRegion members (the opt_in_cset field), causing crashes in future GCs. The solution is to properly filter out remembered set entries of any kind during the optional parts of the gc to already evacuated regions, i.e. also regions in the existing collection set added during incremental GC. Some further evaluation and an example is present in the comments for JDK-8222426. This change builds upon JDK-8223018 and JDK-8200545 also out for review. CR: https://bugs.openjdk.java.net/browse/JDK-8222492 Webrev: http://cr.openjdk.java.net/~tschatzl/8222492/webrev/ Testing: hs-tier1-5, running the crashing test of JDK-8222426 for 5k times without failures (the crash has an occurrence of 1-2 times/1000 runs). Thanks, Thomas

3 4

RFR (M): 8200545: Improve filter for enqueued deferred cards
by Thomas Schatzl 13 May '19

13 May '19

Hi all, can I have reviews for this change that improves the check whether the destination region for a reference has a remembered set that needs updating? This removes the need to use two distinct closures for scanning oops during evacuation, as both the new and the old variant are now equal in performance, so not only simplifying existing code, but also simplifying the follow-up change JDK-8213108 a lot. The idea of this change is to put the information whether a region needs remembered set updates into the InCSetState table. This results in that for every reference we do not need to (re-)calculate the destination region, and then do the walk across several data structures (HeapRegion->HeapRegionRemSet) every time - this simply combines this with the existing calculation of InCSetState. The code adds a new member to InCSetState that indicates whether the region needs remembered set updates. As a second, seperate step, there is a change that renames InCSetState in a imho more understandable G1RegionAttr(ibutes); if wanted I can do this in a separate CR, but here I will just present them as separate webrevs. CR: https://bugs.openjdk.java.net/browse/JDK-8200545 Webrev: http://cr.openjdk.java.net/~tschatzl/8200545/webrev/ (add InCSetState member only) http://cr.openjdk.java.net/~tschatzl/8200545/webrev.rename/ (rename InCSetstate only) http://cr.openjdk.java.net/~tschatzl/8200545/webrev.full/ (full change) Testing: hs-tier1-5, internal perf testing; While testing I found JDK-8222426 - however that one also occurs without the changes, so I think we can go ahead with this regardless. Thanks, Thomas

3 9

RFR: JDK-8222926: Unify cpu related part of shenandoahBarrierSetC1 and fix build when compiler1 is disabled
by Ao Qi 08 May '19

08 May '19

Hi, When I configure using --with-jvm-features=-compiler1, I found build is broken. The main cause is that some shenandoah related part did not consider the situation when compiler1 is disabled. I also found that src/hotspot/cpu/aarch64/gc/shenandoah/shenandoahBarrierSetC1_aarch64.cpp and src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetC1_x86.cpp were basically the same, with only small differences. I unified these two files and moved the result to a c1 related file. I also fixed one "#undef" only when COMPILER1 is true in shenandoahBarrierSetAssembler_x86/aarch64.cpp and one missed include in codeBlob.cpp. Bug: https://bugs.openjdk.java.net/browse/JDK-8222926/ Webrev: http://cr.openjdk.java.net/~aoqi/8222926/webrev.00/ Tested: - build: linux-x86_64-{server, zero, minimal}-{release, fastdebug}, linux-aarch64-server-release - jtreg: linux-x86_64-server-release tier1, linux-aarch64-server-release tier1_gc_shenandoah Thanks, Ao Qi

3 20

RFR: JDK-8222537: Avoid fetching _klass twice in TypeArrayOop::size()
by Roman Kennke 07 May '19

07 May '19

Currently, when calling TypeArrayOop::size(), we end up calling klass() twice: once before calling into size_given_klass() and then again before calling TypeArrayOop::object_size(). This is currently only a minor performance nuisance. With upcoming Shenandoah's elimination of forwarding pointer, loading klass like this is not safe anymore, and therefore we only call size_given_klass(), and must avoid calling naked klass() altogether. Bug: https://bugs.openjdk.java.net/browse/JDK-8222537 Webrev: http://cr.openjdk.java.net/~rkennke/JDK-8222537/webrev.00/ Testing: hotspot_gc_shenandoah with and without the prototype, hotspot/tier1 Can I please get reviews? Thanks, Roman

2 3

RFR (xs): 8221509: Refactor assert( G1CollectedHeap::used() == recalculate_used() ) with better message
by sangheon.kim＠oracle.com 06 May '19

06 May '19

Hi all, Can I have some reviews that replace assert(used() == recalculated_used()) with better message? Those assert messages are not much helpful as it doesn't include any actual values on it so I'm suggesting to include actual values of each and then replace as a macro. CR: https://bugs.openjdk.java.net/browse/JDK-8221509 webrev: http://cr.openjdk.java.net/~sangheki/8221509/webrev.0/ testing: local build Thanks, Sangheon

3 5

RFR (T): 8223018: Rename G1RemSet::*oops_into_collection_set_do methods
by Thomas Schatzl 06 May '19

06 May '19

Hi all, can I have reviews for this change that fixes the G1RemSet::*oops_into_collection_set_do methods to correspond to the naming of the main method scan_rem_set again. CR: https://bugs.openjdk.java.net/browse/JDK-8223018 Webrev: http://cr.openjdk.java.net/~tschatzl/8223018/webrev Testing: local compilation Thanks, Thomas

3 3

RFR: 8222480: Implementation: JEP 351: ZGC: Uncommit Unused Memory
by Per Liden 03 May '19

03 May '19

Please review the implementation for JEP 351: ZGC: Uncommit Unused Memory. Once the review has been completed, I intend to move the JEP to Proposed To Target for JDK 13. The JEP has been updated to reflect the latest state of the implementation. Testing: Passes tier1-3 on all Oracle platformsm and tier1-7 on Linux/x86_64. JEP: https://bugs.openjdk.java.net/browse/JDK-8220347 Bug: https://bugs.openjdk.java.net/browse/JDK-8222480 Webrev: http://cr.openjdk.java.net/~pliden/8222480/webrev.0 /Per

3 7