December 2018 - 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 (XS) 8215221: Serial GC misreports young GC time
by Aleksey Shipilev 06 Mar '19

06 Mar '19

Bug: https://bugs.openjdk.java.net/browse/JDK-8215221 One of our guys pointed out the oddity in Serial GC logs: "Pause Young" includes the timing for Full collection, and wrecks up GCId order as well. This is because GCTraceTime-s are nested on those paths, and putting them in proper order, closer to collect_generation, fixes it. The downside is that some of the cleanup actions are now outside the GCTraceTime. Those look innocuous to omit to me, though. Fix: http://cr.openjdk.java.net/~shade/8215221/webrev.01/ Testing: hotspot tier1, eyeballing GC logs, jdk-submit (running) Thanks, -Aleksey

3 11

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

RFR: 8214799: Add package declaration to each JTREG test case in the gc folder
by Leo Korinth 25 Jan '19

25 Jan '19

Hi, I have added package declarations to all jtreg tests in the gc folder (and sub folders). Previously most tests were not in a package (though some where), making it extremely hard to work with jtreg tests in an IDE. The main change consists of adding a package declaration that corresponds to the directory the test is located in. This also makes it necessary to change @run annotations. Some test cases uses "common" code that is not located in the test lib. Those test cases now need to have a @library annotation that references the jtreg test root. A few test cases had package visible classes with the same name in the same directory. That did work before, but does not work anymore, so I have prefixed those package visible classes with the public class name. class TestVerifySilentlyRunSystemGC class TestVerifySubSetRunSystemGC class TestEagerReclaimHumongousRegionsReclaimRegionFast class TestEagerReclaimHumongousRegionsClearMarkBitsReclaimRegionFast class TestEagerReclaimHumongousRegionsWithRefsReclaimRegionFast Where classes have been referenced using string literals, I have tried to use a .class.getName() instead (where possible). I have tried to update copyrights to all files (also adding a missing comma after the second year in some files), and I did add missing copyright to TestFromCardCacheIndex. known cons: - needs a package line in each java file - needs an extra library line in the test if the test is using other files. - sometimes needs a run tag improvements: + can use eclipse or other IDEs without creating a workspace per test (I guess it will be a significant improvement for users of emacs and vi as well) + can use test files located in other directories After this change, I have several big cleanups that are easy to do with a working IDE. Enhancement: https://bugs.openjdk.java.net/browse/JDK-8214799 Webrev: http://cr.openjdk.java.net/~lkorinth/8214799/00/ Testing: passed: 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 now running: mach5 remote-test --build-id 2018-12-05-1132377.leo.korinth.hs --build-profiles linux-x64-debug -j tier1,tier2,tier3 Thanks, Leo

3 9

RFR(XS): 8214235: assertion in collectedHeap.cpp: attempt to clean empty remainder
by Boris Ulasevich 08 Jan '19

08 Jan '19

Hi all, Please review a simple fix: do not fill remaining tlab memory when the remainder is empty. https://bugs.openjdk.java.net/browse/JDK-8214235 http://cr.openjdk.java.net/~bulasevich/8214235/webrev.01 Thanks, Boris

4 15

RFR(S): 8215935: G1 Old Gen MemoryPool CollectionUsage.used values don't reflect mixed GC results
by Hohensee, Paul 27 Dec '18

27 Dec '18

Hi, Please review CSR JDK-8215935<https://bugs.openjdk.java.net/browse/JDK-8215935>. This is the CSR for JDK-8215934<https://bugs.openjdk.java.net/browse/JDK-8215934>, which is the jdk8u backport issue for JDK-8195115<https://bugs.openjdk.java.net/browse/JDK-8195115>, which latter was fixed in jdk11. The corresponding jdk11 CSR is JDK-8196719<https://bugs.openjdk.java.net/browse/JDK-8196719>. Once/If approved, I’ll post a backport webrev for review. The backport is a bit different from the original patch. Thanks, Paul

1 0

RFR: 8214201: Make PtrQueueSet completed buffer list private
by Kim Barrett 26 Dec '18

26 Dec '18

Please review this refactoring of PtrQueueSet and its derivations to make the data members for the completed buffer list private (rather than protected), and clean up the associated API. This also merges some otherwise nearly duplicated code from DirtyCardQueueSet and SATBMarkQueueSet into PtrQueueSet. Moved DirtyCardQueueSet::get_completed_buffer() to PtrQueueSet, and changed SATBMarkQueueSet to use it too, rather than having its own inline near copy. Note that get_completed_buffer clears the _process_completed_buffers flag when the list becomes empty, as needed for the SATB case; the DCQS case doesn't care. Also take care to NULL out the next field of the returned buffer; currently no code cares, but in the future we might add asserts when enqueuing or freeing. Added PtrQueueSet::abandon_completed_buffers(). Changed SATB and DC queue sets to use it rather than duplicate code. Made some more naming updates for consistent nomenclature. We now consistently use "completed_buffer" rather than "complete_buffer" or "completed". - enqueue_complete_buffer() => enqueue_completed_buffer() - _process_completed => _process_completed_buffers - set_process_completed() => set_process_completed_buffers() (Note: Already had process_completed_buffers()). CR: https://bugs.openjdk.java.net/browse/JDK-8214201 Webrev: http://cr.openjdk.java.net/~kbarrett/8214201/open.00/ Testing: mach5 tier1-5.

3 6

8215897: Build broken on zero after JDK-8211424
by Stefan Johansson 22 Dec '18

22 Dec '18

Hi, Please review this fix for a build failure on Zero (or platforms excluding G1 or Parallel). Bug: https://bugs.openjdk.java.net/browse/JDK-8215897 Webrev: http://cr.openjdk.java.net/~sjohanss/8215897/00/ Summary The new whitebox methods for the AllocateOldGenAt feature was guarded by (G1 | Parallel), and this will not work if one of the GCs are not present in the build. Testing Built the previously failing platforms. Thanks, Stefan

2 1

8215898: Build broken on 32-bit after JDK-8211425
by Stefan Johansson 22 Dec '18

22 Dec '18

Hi all, Please review this small fix for a build failure: Bug: https://bugs.openjdk.java.net/browse/JDK-8215898 Webrev: http://cr.openjdk.java.net/~sjohanss/8215898/00/ Summary The build failure is caused by always setting UseCompressedOops and UseCompressedClassPointers to false when AllocateOldGenAt is used. This needs to be guarded with LP64_ONLY. Testing Built the previously failing platforms and they now pass. Thanks, Stefan

1 0