September 2020 - hotspot-gc-dev

Re: RFR: 8240556: Abort concurrent mark after effective eager reclamation of humongous objects
by Albert Mingkun Yang 18 Sep '20

18 Sep '20

On Tue, 15 Sep 2020 12:40:46 GMT, Thomas Schatzl <tschatzl(a)openjdk.org> wrote: > Hi all, > > please review this change that implements concurrent mark abort as proposed by Liang Mao from Alibaba. > > Basically, if at the end of the concurrent start pause we notice that we went below the IHOP threshold (and it has been > a concurrent start pause caused by humongous allocation), instead of scheduling a mark operation, we schedule a > "concurrent undo" operation that undoes the changes. Regarding the removal of the > test/hotspot/jtreg/gc/stress/jfr/TestStressBigAllocationGCEventsWithG1.java test: it only ever accidentally worked in > G1. G1 never sent the AllocationRequiringGC events for GCs caused by going over the IHOP threshold for humongous > allocations that the test actually expects. That test previously only worked because G1 could not reclaim the > humongous objects fast enough so crossing the IHOP threshold causes a full concurrent mark. Allocations during that > concurrent mark do not cause a GC that can reclaim these objects, so ultimately some young GC that sends the > AllocationRequiringGC event will be sent. With concurrent undo this is not guaranteed any more, i.e. only in > environments where concurrent undo is slow (and we'll improve it soon) this test works. The test is too timing > dependent, and checking for the wrong thing in this case, so removing it. Testing: tbd. src/hotspot/share/gc/g1/g1ConcurrentMarkThread.hpp line 112: > 110: bool idle() const; > 111: bool started() const; > 112: bool in_progress() const; What's the motivation of moving them out of the header? Since they are quite small, placing them in the header makes inlining possible and desirable. src/hotspot/share/gc/g1/g1ConcurrentMarkThread.hpp line 43: > 41: G1ConcurrentMark* _cm; > 42: > 43: enum ServiceState : uint { Why `: uint`? The same question for `enum Command` below as well. ------------- PR: https://git.openjdk.java.net/jdk/pull/177

3 3

RFR: 8244505: G1 pause time ratio calculation does not consider Remark/Cleanup
by Ivan Walulya 18 Sep '20

18 Sep '20

The computation for long and short term pause time ratios does not include Remark/Cleanup pause times. Isolate updates to _prev_collection_pause_end_ms from update_recent_gc_times(), then have all gc pauses (including Remark/Cleanup) recorded by update_recent_gc_times(). Patch also includes trivial clean ups to g1Policy.* files. ------------- Commit messages: - 8244505: G1 pause time ratio calculation does not consider Remark/Cleanup Changes: https://git.openjdk.java.net/jdk/pull/183/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=183&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8244505 Stats: 53 lines in 4 files changed: 26 ins; 14 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/183.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/183/head:pull/183 PR: https://git.openjdk.java.net/jdk/pull/183

4 16

RFR: 8252041: G1: Fix incorrect uses of HeapRegionManager::max_length
by Thomas Schatzl 17 Sep '20

17 Sep '20

Hi all, can I have reviews for this change that fixes some uses of HeapRegionManager::max_regions()/max_expandable_regions() in our code to use the correct variant? HeapRegionManager::max_length gives the absolute maximum number of regions reserved for the heap based on the reservation. HeapRegionmanager::max_expandable_regions() returns the maximum number of regions the heap can grow to (can be committed). Typically they are exchangeable, but if the reservation is larger than what -Xmx allows (like when file-mapping old gen), this is not the case, and causes assertions. In some cases the existing region manager code could (if it reserved more than -Xmx) give back more than -Xmx allows. Testing: tier1-5. ------------- Commit messages: - Initial import Changes: https://git.openjdk.java.net/jdk/pull/178/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=178&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252041 Stats: 45 lines in 6 files changed: 15 ins; 0 del; 30 mod Patch: https://git.openjdk.java.net/jdk/pull/178.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/178/head:pull/178 PR: https://git.openjdk.java.net/jdk/pull/178

5 12

RFR: 8252196: ZGC: TestUncommit.java fails due to "Exception: Uncommitted too fast" again(2)
by Per Lidén 17 Sep '20

17 Sep '20

This test has failed a few times because the timeAfterAlloc timestamp (the time when uncommit can start to happen) is taken at the wrong place. We should take this timestamp before we call allocate(), since the allocation could cause ZPageCache::_last_commit to be set, which will dictate when uncommit can happen. This PR moves and renames timeAfterAlloc accordingly. Testing: gc/z/TestUncommit.java executed multiple times on all platforms. ------------- Commit messages: - 8252196: ZGC: TestUncommit.java fails due to "Exception: Uncommitted too fast" again(2) Changes: https://git.openjdk.java.net/jdk/pull/179/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=179&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252196 Stats: 3 lines in 1 file changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/179.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/179/head:pull/179 PR: https://git.openjdk.java.net/jdk/pull/179

3 3

RFR: 8253232: G1Analytics::compute_pause_time_ratios() uses wrong pause times in calculation
by Ivan Walulya 17 Sep '20

17 Sep '20

Changed to consider only gc pause times within the long time interval. Testing: Tier 1 ------------- Commit messages: - 8253232: G1Analytics::compute_pause_time_ratios() uses wrong pause times Changes: https://git.openjdk.java.net/jdk/pull/210/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=210&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253232 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/210.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/210/head:pull/210 PR: https://git.openjdk.java.net/jdk/pull/210

4 6

RFR: 8253244: Shenandoah: cleanup includes in Shenandoah root processor files
by Zhengyu Gu 16 Sep '20

16 Sep '20

Many roots were moved to oop storages, Shenandoah root processor no longer references to StringTable, systemDictionary and ResolvedMethodTable, etc. ------------- Commit messages: - JDK-8253244 Changes: https://git.openjdk.java.net/jdk/pull/206/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=206&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253244 Stats: 7 lines in 2 files changed: 0 ins; 6 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/206.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/206/head:pull/206 PR: https://git.openjdk.java.net/jdk/pull/206

2 4

RFR: 8253226: Shenandoah: remove unimplemented ShenandoahStrDedupQueue::verify
by Aleksey Shipilev 16 Sep '20

16 Sep '20

`ShenandoahStrDedupQueue` defers to super-class that in turn calls `verify_impl()`. `verify()` is left unused and unimplemented. Static analysis complains about it. Testing: Linux x86_64 fastdebug tier1_gc_shenandoah ------------- Commit messages: - 8253226: Shenandoah: remove unimplemented ShenandoahStrDedupQueue::verify Changes: https://git.openjdk.java.net/jdk/pull/203/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=203&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253226 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/203.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/203/head:pull/203 PR: https://git.openjdk.java.net/jdk/pull/203

3 4

RFR: 8253224: Shenandoah: ShenandoahStrDedupQueue destructor calls virtual num_queues()
by Aleksey Shipilev 16 Sep '20

16 Sep '20

Static analysis complains that `ShenandoahStrDedupQueue` destructor calls virtual `num_queues()`. We can just do the `_nv` version to make this right. Testing: hotspot_gc_shenandoah ------------- Commit messages: - 8253224: Shenandoah: ShenandoahStrDedupQueue destructor calls virtual num_queues() Changes: https://git.openjdk.java.net/jdk/pull/202/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=202&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253224 Stats: 4 lines in 2 files changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/202.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/202/head:pull/202 PR: https://git.openjdk.java.net/jdk/pull/202

3 4

RFR: 8253173: Print heap before and after GC lacks a newline
by Stefan Karlsson 16 Sep '20

16 Sep '20

When GC logging was converted to Unified Logging a newline went missing. - st->print_cr("{Heap before GC invocations=%u (full %u):", - heap()->total_collections(), - heap()->total_full_collections()); vs + LogHandle(gc, heap) log; + if (log.is_trace()) { + log.trace("Heap before GC invocations=%u (full %u):", heap()->total_collections(), heap()->total_full_collections()); This gives lines like this: GC(29) Heap after GC invocations=30 (full 30): ZHeap used 20M, capacity 2014M, max capacity 30718M GC(29) Metaspace used 6779K, capacity 6857K, committed 7040K, reserved 1056768K GC(29) class space used 574K, capacity 620K, committed 640K, reserved 1048576K Instead of: GC(29) Heap after GC invocations=30 (full 30): GC(29) ZHeap used 20M, capacity 2014M, max capacity 30718M GC(29) Metaspace used 6779K, capacity 6857K, committed 7040K, reserved 1056768K GC(29) class space used 574K, capacity 620K, committed 640K, reserved 1048576K In our hs_err files we print this with a newline: st.print_cr("{Heap %s GC invocations=%u (full %u):", before ? "before" : "after", heap->total_collections(), heap->total_full_collections()); I propse that we add this missing newline. ------------- Commit messages: - Add print heap at GC newline and move the code Changes: https://git.openjdk.java.net/jdk/pull/187/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=187&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253173 Stats: 39 lines in 3 files changed: 15 ins; 22 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/187.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/187/head:pull/187 PR: https://git.openjdk.java.net/jdk/pull/187

6 6

RFR: 8253219: Epsilon: clean up unnecessary includes
by Aleksey Shipilev 16 Sep '20

16 Sep '20

Static analysis shows a few #include statements in gc/epsilon that are not needed. Testing: Linux x86_64 {release, fastdebug, slowdebug} x {+PCH, -PCH} ------------- Commit messages: - 8253219: Epsilon: clean up unnecessary includes Changes: https://git.openjdk.java.net/jdk/pull/198/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=198&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253219 Stats: 3 lines in 2 files changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/198.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/198/head:pull/198 PR: https://git.openjdk.java.net/jdk/pull/198

5 8