June 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 (M/L): 8247928: Refactor G1ConcurrentMarkThread for mark abort (JDK-8240556)
by Thomas Schatzl 04 Sep '20

04 Sep '20

Hi all, can I have reviews for this change that refactors the G1ConcurrentMarkThread class to prepare it better for mark abort (JDK-8240556). So the idea in the latter is to abort the concurrent cycle if after concurrent start gc we find that old gen occupancy actually went down below the IHOP again due to e.g. eager reclaim. To do that, G1 needs to scrub any marks on the next bitmap during the gc pause. The current, original change just performs this work in the pause, which is very slow, so the idea is to do the bitmap cleaning concurrently instead. The problem is that G1ConcurrentMarkThread is a mess and you can't easily "jump" to the end of the current marking. Additionally, the code was worth refactoring without that requirement anyway ;) This change refactors the code so that it is much easier to do add a second path through the concurrent cycle. Overall, there are two options to do that: 1) provide an explicit state machine for the concurrent marking so that you can jump to the end easily. 2) provide building blocks that can be easily put together to implement the second path. While 1) works (there is a sample POC at http://cr.openjdk.java.net/~tschatzl/8247928/webrev.sm/) I found that it is much more code and less understandable than just building two paths through the marking cycle as in the "abort marking" case we only need to do the very tail end of the regular full marking cycle, so I propose this option for review. This is refactoring (almost) only, the additional path should be added with JDK-8240556. Almost because I changed two things: - the concurrent mark (control) thread does not do any marking any more for a long time (long long ago it did root scanning/marking), so I removed the _vtime_mark_accum accumulator. - the "Concurrent Mark" finish message is now only printed at the end of all marking, not every iteration. First, the restart case is very rare so probably anyone parsing it will not handle this case correctly, and the contents (times) of that finish message are confusing, which means most likely anyone handling it will likely do the wrong thing. The "Concurrent Mark Restart for Mark Overflow" remains to indicate a restart. Based on JDK-8248221 also out for review. CR: https://bugs.openjdk.java.net/browse/JDK-8247928 Webrev: http://cr.openjdk.java.net/~tschatzl/8247928/webrev/ Testing: tier1-5, a few local jtreg runs of the gc directory Thanks, Thomas

3 8

RFR: 8188055: (ref) Add Reference.refersTo predicate
by Kim Barrett 02 Sep '20

02 Sep '20

[Note review on both core-libs and hotspot-gc-dev lists; try not to lose either when replying.] Please review a new function: java.lang.ref.Reference.refersTo. This function is needed to test the referent of a Reference object without artificially extending the lifetime of the referent object, as may happen when calling Reference.get. Some garbage collectors require extending the lifetime of a weak referent when accessed, in order to maintain collector invariants. Lifetime extension may occur with any collector when the Reference is a SoftReference, as calling get indicates recent access. This new function also allows testing the referent of a PhantomReference, which can't be accessed by calling get. The new function uses a native method whose implementation is in the VM so it can use the Access API. It is the intent that this function will be intrinsified by optimizing compilers like C2 or graal, but that hasn't been implemented yet. Bear that in mind before rushing off to change existing uses of Reference.get. CR: https://bugs.openjdk.java.net/browse/JDK-8188055 https://bugs.openjdk.java.net/browse/JDK-8241029 (CSR) Webrev: https://cr.openjdk.java.net/~kbarrett/8188055/open.04/ Testing: mach5 tier1 Locally (linux-x64) verified the new test passes with various garbage collectors.

8 18

RFR: 8248266: ZGC: TestUncommit.java fails due to "Exception: Uncommitted too fast" again
by Per Liden 18 Aug '20

18 Aug '20

Hi, The test gc/z/TestUncommit.java sometimes fail because of bad timing, caused by what appears to be an heavily loaded machine so the test thread doesn't get to execute in a timely manner. I've restructured the test a bit, to be less sensitive to this. Instead of sleeping and then checking if uncommit has happened, the test will now wait until uncommit happens and record the time. When checking the time it now uses TIMEOUT_FACTOR to control how strict the check should be. To keep things simple, I've also broken out the part that tests with uncommit disabled into a separate test. The change in ZPage/ZPageCache rounds up the last_used/last_committed timestamps to the nearest second. Without this we will always be rounding the time down, which means we can sometimes uncommit one second too early, which is wrong but not a big problem, but this can also cause TestUncommit.java to fail with "Uncommitted too fast". Bug: https://bugs.openjdk.java.net/browse/JDK-8248266 Webrev: http://cr.openjdk.java.net/~pliden/8248266/webrev.0 Testing: Ran TestUncommit 100+ times on all Oracle-platforms. /Per

3 5

RFR (M): 8245721: Refactor the TaskTerminator
by Thomas Schatzl 17 Aug '20

17 Aug '20

Hi all, can I have reviews for this refactoring of the (OWST) TaskTerminator to make the algorithm more understandable. The original implementation imho suffers from two issues: - manual lock() and unlock() of the _blocker synchronization lock everywhere, distributed around two separate methods. - interspersing the actual spinning code somewhere inlined inbetween. This change tries to hopefully successfully make reasoning about the code *much* easier by different separation of these two methods, and using scoped locks. The final structure of the code has been intensively tested to not cause a regression in performance, however it made a few "obvious" further refactorings undesired due to signficant perf regressions. I believe I found a good tradeoff here, but I am of course open to improvements :) I tried to sketch a few of those ultimately unsuccessful attempts in the CR. CR: https://bugs.openjdk.java.net/browse/JDK-8245721 Webrev: http://cr.openjdk.java.net/~tschatzl/8245721/webrev/ Testing: tier1-5, many many perf rounds, many tier1-X rounds with other patches Thanks, Thomas

4 12

How to run specific part of the SerialGC in a new thread?
by Ofir Gordon 03 Aug '20

03 Aug '20

Hello, I'm trying to run a specific procedure within the SerialGC in a separate thread, i.e. replace the call for the procedure with a creation of a new thread that will run the entire procedure and then return to the main thread and continue (the main thread should wait to the splitted thread). What is the correct way to do it? Is there any threads mechanism that the vm uses in order to split tasks to new threads? I tried to simply use the pthread library, but when I add calls to pthread methods (like pthread_create) the compilation fails (with segfault..) I'll appreciate your help, Thanks, Ofir

2 8

RFR (M): 8210462: Fix remaining mentions of initial mark
by Thomas Schatzl 17 Jul '20

17 Jul '20

Hi all, can I have reviews for this change that removes remaining use of the "initial mark pause" to the "newer" concurrent start in the code? This is mostly a straightforward s/initial mark/concurrent start/ replacement. CR: https://bugs.openjdk.java.net/browse/JDK-8210462 Webrev: http://cr.openjdk.java.net/~tschatzl/8210462/webrev/ Testing: tier1-5 Thanks, Thomas

3 7

[16] RFR: 8248391: Unify handling of all OopStorage instances in weak root processing
by Erik Österlund 16 Jul '20

16 Jul '20

Hi, Today, when a weak OopStorage is added, you have to plug it in explicitly to ZGC, Shenandoah and the WeakProcessor, used by Shenandoah, Serial, Parallel and G1. Especially when the runtime data structure associated with an OopStorage needs a notification when oops die. Then you have to explicitly plug in notification code in various places in GC code. It would be ideal if this process could be completely automated. This patch allows each OopStorage to have an associated notification function. This is a callback function into the runtime, stating how many oops have died this GC cycle. This allows runtime data structures to perform accounting for how large part of the data structure needs cleaning, and whether to trigger such cleaning or not. So the interface between the GCs to the OopStorage is that during weak processing, the GC promises to call the callback function with how many oops died. Some shared infrastructure makes this very easy for the GCs. Weak processing now uses the OopStorageSet iterators across all GCs, so that adding a new weak OopStorage (even with notification functions) does not require touching any GC code. Kudos to Zhengyu for providing some Shenandoah code for this, and StefanK for pre-reviewing it. Also, I am about to go out of office now, so StefanK promised to take it from here. Big thanks for that! CR: https://bugs.openjdk.java.net/browse/JDK-8248391 Webrev: http://cr.openjdk.java.net/~eosterlund/8248391/webrev.00/ Thanks, /Erik

6 8

RFR 8247670: Shenandoah: deadlock during class unloading OOME
by Zhengyu Gu 10 Jul '20

10 Jul '20

The deadlock is caused by one thread holding per-nmethod lock, then encountering evac-oom. At the same time, another thread entering evac-oom scope, then acquiring the same per-nmethod lock. The first thread expects the second thread to see evac-oom and exit the scope, but the second thread is blocked on acquiring per-nmethod lock. The solution is to introduce an abortable locker on per-nmethod lock. If the second thread can not acquire the lock, but see evac-oom, it simply aborts, so it can exit evac-oom scope. The solution does come with penalties: If the second thread is a Java thread (via nmethod entry barrier), the nmethod will be deopt. If the second thread is worker, it causes current code root processing to abort, then restart. Bug: https://bugs.openjdk.java.net/browse/JDK-8247670 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8247670/webrev.00/ Test: hotspot_gc_shenandoah (x86_64 and aarch64) Thanks, -Zhengyu

2 3