September 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(S) : 8246494 : introduce vm.flagless at-requires property
by Igor Ignatyev 09 Mar '21

09 Mar '21

http://cr.openjdk.java.net/~iignatyev//8246494/webrev.00 > 70 lines changed: 66 ins; 0 del; 4 mod Hi all, could you please review the patch which introduces a new @requires property to filter out the tests which ignore externally provided JVM flags? the idea behind this patch is to have a way to clearly mark tests which ignore flags, so a) it's obvious that they don't execute a flag-guarded code/feature, and extra care should be taken to use them to verify any flag-guarded changed; b) they can be easily excluded from runs w/ flags. @requires and VMProps allows us to achieve both, so it's been decided to add a new property `vm.flagless`. `vm.flagless` is set to false if there are any XX flags other than `-XX:MaxRAMPercentage` and `-XX:CreateCoredumpOnCrash` (which are known to be set almost always) or any X flags other `-Xmixed`; in other words any tests w/ `@requires vm.flagless` will be excluded from runs w/ any other X / XX flags passed via `-vmoption` / `-javaoption`. in rare cases, when one still wants to run the tests marked by `vm.flagless` w/ external flags, `vm.flagless` can be forcefully set to true by setting any value to `TEST_VM_FLAGLESS` env. variable. this patch adds necessary common changes and marks common tests, namely Scimark, GTestWrapper and TestNativeProcessBuilder. Component-specific tests will be marked separately by the corresponding subtasks of 8151707[1]. please note, the patch depends on CODETOOLS-7902336[2], which will be included in the next jtreg version, so this patch is to be integrated only after jtreg5.1 is promoted and we switch to use it by 8246387[3]. JBS: https://bugs.openjdk.java.net/browse/JDK-8246494 webrev: http://cr.openjdk.java.net/~iignatyev//8246494/webrev.00 testing: marked tests w/ different XX and X flags w/ and w/o TEST_VM_FLAGLESS env. var, and w/o any flags [1] https://bugs.openjdk.java.net/browse/JDK-8151707 [2] https://bugs.openjdk.java.net/browse/CODETOOLS-7902336 [3] https://bugs.openjdk.java.net/browse/JDK-8246387 Thanks, -- Igor

3 5

RFR: 8234930: Use MAP_JIT when allocating pages for code cache on macOS
by Anton Kozlov 15 Dec '20

15 Dec '20

Please review an updated RFR from https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/041… On macOS, MAP_JIT cannot be used with MAP_FIXED[1]. So pd_reserve_memory have to provide MAP_JIT for mmap(NULL, PROT_NONE), the function was made aware of exec permissions. For executable and data regions, pd_commit_memory only unlocks the memory with mprotect, this should make no difference compared with old code. For data regions, pd_uncommit_memory still uses a new overlapping anonymous mmap which returns pages to the OS and immediately reflects this in diagnostic tools like ps. For executable regions it would require MAP_FIXED|MAP_JIT, so instead madvise(MADV_FREE)+mprotect(PROT_NONE) are used. They should also allow OS to reclaim pages, but apparently this does not happen immediately. In practice, it should not be a problem for executable regions, as codecache does not shrink (if I haven't missed anything, by the implementation and in principle). Tested: * local tier1 * jdk-submit * codesign[2] with hardened runtime and allow-jit but without allow-unsigned-executable-memory entitlements[3] produce a working bundle. (adding GC group as suggested by @dholmes-ora) [1] https://github.com/apple/darwin-xnu/blob/master/bsd/kern/kern_mman.c#L227 [2] codesign \ --sign - \ --options runtime \ --entitlements ents.plist \ --timestamp \ $J/bin/* $J/lib/server/*.dylib $J/lib/*.dylib [3] <?xml version="1.0" encoding="UTF-8"?> <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> <plist version="1.0"> <dict> <key>com.apple.security.cs.allow-jit</key> <true/> <key>com.apple.security.cs.disable-library-validation</key> <true/> <key>com.apple.security.cs.allow-dyld-environment-variables</key> <true/> </dict> </plist> ------------- Commit messages: - Use MAP_JIT for CodeCache pages Changes: https://git.openjdk.java.net/jdk/pull/294/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=294&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8234930 Stats: 65 lines in 15 files changed: 27 ins; 0 del; 38 mod Patch: https://git.openjdk.java.net/jdk/pull/294.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/294/head:pull/294 PR: https://git.openjdk.java.net/jdk/pull/294

5 42

Re: RFR: 8234930: Use MAP_JIT when allocating pages for code cache on macOS
by Stefan Karlsson 02 Dec '20

02 Dec '20

On Mon, 28 Sep 2020 11:16:46 GMT, Thomas Stuefe <stuefe(a)openjdk.org> wrote: >> Hi Anton, >> >> I am sorry, but I dislike the API changes to os::xxx(). As it is now, this API is quite convoluted, undocumented and >> inconsistent. There are ongoing efforts to clean this up (see eg JDK-8253650, JDK-8253638 and JDK-8253683). This mess >> carries over into the upper layers too, e.g. ReservedSpace. About the API changes: >> >> - Why does os::uncommit_memory() now require "exec" as parameter? I know why you do it, but semantically it has no >> meaning. I should be able to uncommit memory without knowing the exact flags the mapping had been established with. So >> now the user has to carry this information and provide it back to the API when uncommitting? Right now probably all >> uncommits happen in areas where the exec information is implicit by its usage, but who says this is always the case? >> >> - We now have to specify "exec" as parameter to os::reverve_memory(). This is another new restriction - before, I could >> theoretically reserve one mapping and commit various parts of it with and without exec flag. With this change, the >> whole mapping has to be either exec or !exec. >> Can we on MacOS not use mmap at os::commit_memory, like we do on other platforms? In os::commit_memory, the exec >> parameter exists but on MacOS it is ignored. It would be more consistent with other platforms if the exec parameter >> were honored in commit, not requiring a new one at reserve. And I see mmap is used in uncommit, so changing mappings >> via overlapped mmap seems to work? Thanks, Thomas > >> I am sorry, but I dislike the API changes to os::xxx(). As it is now, this API is quite convoluted, undocumented and >> inconsistent. > > Just to clarify that I did not mean your change makes the API convoluted and inconsistent but that it has been that way > already, for a long time. It is a constant headache to get right, especially in ports whose memory management is a bit > non-standard, e.g. on AIX. @tstuefe My patch to remove MAP_FIXED from the memory reservation path should make it possible to revert all the os::reserve_memory changes in this patch. ------------- PR: https://git.openjdk.java.net/jdk/pull/294

4 14

RFR: 8252103: Parallel heap inspection for ParallelScavengeHeap
by Lin Zang 05 Nov '20

05 Nov '20

- Parallel heap iteration support for PSS - JBS: https://bugs.openjdk.java.net/browse/JDK-8252103 ------------- Commit messages: - 8252103: Parallel heap inspection for ParallelScavengeHeap Changes: https://git.openjdk.java.net/jdk/pull/25/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=25&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252103 Stats: 165 lines in 6 files changed: 165 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/25.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/25/head:pull/25 PR: https://git.openjdk.java.net/jdk/pull/25

5 70

RFR: 8251158: Implementation of JEP 387: Elastic Metaspace
by Thomas Stuefe 20 Oct '20

20 Oct '20

Hi all, this is the continuation of the ongoing review for the JEP387 implementation (last rounds see [1] [2]). Sorry for the delay, had vacation then the entrance of Skara delayed things a bit. For the delta diff please see [3]. This is the first time I do a large PR after Skara, so if something is wrong please bear with me. I cannot answer all feedback individually in this PR body, but I incorporated almost all into the new revision. What changed since the last version: - I renamed most metaspace files back to the original naming scheme or to something similar, hopefully capturing the group consent. - I changed the way allocation guards are checked if MetaspaceGuardAllocations is enabled. Before, I would test for overwrites upon CLD destruction, but since that check was subject to VerifyMetaspaceInterval it only ran for every nth class loader which made it rather pointless. Now I run it always. - I also improved the printout on block corruption, and log block corruption unconditionally before asserting. - I also fixed up and commented the death test which tests for allocation overwriters (test_allocationGuard.cpp) Side note, I find the corruption check very useful but if you guys think it is too much I still can remove the feature. - In ChunkManager::purge() I improved the comments after discussions with Leo. - I fixed a bug with VerifyMetaspaceInterval: if set to 1 the "SOMETIMES" sections were supposed to fire always, but due to a one-off error they only fired every second time. Now, if -XX:VerifyMetaspaceInterval=1, the checks really run every time. - Fixed indentation issues as Leo requested - Rewrote the condition and the assert in VirtualSpaceList::allocate_root_chunk() as Leo requested - I removed the "can_purge" logic from VirtualSpaceList. The list does not need to know. It just should iterate all nodes and attempt purging, and if a node does not own its ReservedSpace, it refuses to be purged. That is simpler and more flexible since it allows us to have list with purge-able and non-purge-able nodes. - and various smaller fixes, mainly on request of Leo. @lkorinth: > VirtualSpaceNode.hpp > >102 // Start pointer of the area. >103 MetaWord* const _base; > >How does this differ from _rs._base? Really needed? > >105 // Size, in words, of the whole node >106 const size_t _word_size; > >Can we not calculate this from _rs.size()? You are right, _base and _word_size are directly related to the underlying space. But I'd prefer to leave it the way it is. Mainly because ReservedSpace::_base and ::_size are nonconst and theoretically can change under me. It is highly improbable but I'd like to know. Note that VirtualSpaceNode::verify checks that. Should we clean up ReservedSpace at some point and make those members const - as they should be - then I would rewrite this as you suggest. Thanks, again, for all your review work! ------ [1] https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/041… [2] https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-September/… [3] https://github.com/openjdk/jdk/commit/731f795bc0c1c502dc6cac8f866ff45a15bdd… ------------- Commit messages: - Remove trailing whitespaces - JEP387 review feedback round 2 - Review version 2 (2020-09-04) Changes: https://git.openjdk.java.net/jdk/pull/336/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=336&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8251158 Stats: 23038 lines in 170 files changed: 14916 ins; 6912 del; 1210 mod Patch: https://git.openjdk.java.net/jdk/pull/336.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/336/head:pull/336 PR: https://git.openjdk.java.net/jdk/pull/336

7 77

RFR: 8252221: Use multiple workers for Parallel GC pre-touching
by Amit Pawar 15 Oct '20

15 Oct '20

Please review this change. ------------- Commit messages: - 8252221: Improves JVM startup time for ParallelGC Pretouch using multiple threads Changes: https://git.openjdk.java.net/jdk/pull/180/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=180&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252221 Stats: 99 lines in 7 files changed: 82 ins; 4 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/180.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/180/head:pull/180 PR: https://git.openjdk.java.net/jdk/pull/180

4 28

RFR: 8253857: Shenandoah: Bugs in ShenandoahEvacOOMHandler related code
by Zhengyu Gu 09 Oct '20

09 Oct '20

There are a couple of bugs in ShenandoahEvacOOMHandler related code. 1) ShenandoahEvacOOMHandler::register_thread() It does not do OOM check before blindly cmpxchg in threads_in_evac. The cmpxchg can success even OOM is in progress, that results the thread to continue evacuate objects. 2) When a nested OOM scope encounters OOM, it does not decrease threads_in_evac count before entering wait_for_no_evac_threads() loop, it deadlocks itself. ------------- Commit messages: - 8253857: Shenandoah: Bugs in ShenandoahEvacOOMHandler related code Changes: https://git.openjdk.java.net/jdk/pull/439/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=439&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253857 Stats: 26 lines in 2 files changed: 8 ins; 7 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/439.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/439/head:pull/439 PR: https://git.openjdk.java.net/jdk/pull/439

2 4

RFR: 8252105: parallel heap inspection for ZCollectedHeap
by Lin Zang 05 Oct '20

05 Oct '20

- enable parallel heap inspection for ZCollectedHeap - preliminary evaluation: Time of jmap histo on 8GB heap with ~5GB objects * before: 7.103s * after : 2.734s (with 4 parallel threads) ------------- Commit messages: - 8252105: parallel heap inspection for ZCollectedHeap Changes: https://git.openjdk.java.net/jdk/pull/103/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=103&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252105 Stats: 380 lines in 6 files changed: 380 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/103.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/103/head:pull/103 PR: https://git.openjdk.java.net/jdk/pull/103

2 8

RFR: 8253815: Remove unused HeapRegionManager::_num_committed from SA
by Stefan Johansson 30 Sep '20

30 Sep '20

While doing some refactoring I wanted to move HeapRegionManager::_num_committed and realized that I needed to update the SA. After some looking around it turns out that it is unused and I can remove the numCommittedField from the HeapRegionManager class in the SA. Local build and test looks good and running tier1 and some svc-testing to ensure I haven't missed anything. ------------- Commit messages: - 8253815: Remove unused HeapRegionManager::_num_committed from SA Changes: https://git.openjdk.java.net/jdk/pull/408/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=408&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253815 Stats: 8 lines in 2 files changed: 0 ins; 8 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/408.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/408/head:pull/408 PR: https://git.openjdk.java.net/jdk/pull/408

3 7