On Thu, 22 Dec 2022 02:04:30 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
> Please review this change to WhiteBox and some tests involving G1 concurrent > GCs. > > Some tests currently use WhiteBox.g1StartConcMarkCycle() to trigger a > concurrent GC. Many of them follow it with a loop waiting for a concurrent > cycle to not be in progress. A few also preceed that call with a similar > loop, since that call does nothing and returns false if a concurrent cycle is > already in progress. Those tests typically want to ensure there was a > concurrent cycle that was started after some setup. > > The failing test calls that function, asserting that it returned true, e.g. a > new concurrent cycle was started. > > There are various problems with this, due to races with concurrent cycles > started automatically, and due to possibly aborted (by full GCs) concurrent > cycles, > making some of these tests unreliable in some configurations. > > For example, the test failure associated with this bug intermittently arises > when running with `-Xcomp`, triggering a concurrent cycle before the explicit > request by the test, causing the explicit request to fail (because there is > already one in progress), failing the assert. Waiting for there not to be an > in-progress cycle before the explicit request just narrows the race window. > > We have a different mechanism in WhiteBox for controlling concurrent cycles, > the concurrent GC breakpoint mechanism. By adding a counter specifically for > such cycles, we can use GC breakpoints to ensure only the concurrent cycles > the test wants are occurring, and can verify they completed successfully. > > So we change tests using WhiteBox.g1StartConcMarkCycle() to instead use GC > breakpoints, along with the new WhiteBox.g1CompletedConcurrentMarkCycles() to > avoid racing request problems and to detect aborted cycles. Since it is no > longer used, WhiteBox.g1StartConcMarkCycle() is removed. The test that began > this adventure (TestConcMarkCycleWB.java) is also removed, since it no longer > serves any purpose, having been purely a test of that removed function. > > Testing: > mach5 tier1-7 - running the changed tests on a variety of platforms with a > variety of configurations. This pull request has now been integrated. Changeset: b743519b Author: Kim Barrett <kbarr...@openjdk.org> URL: https://git.openjdk.org/jdk20/commit/b743519ba911a254669fa8a96e6006c14e3f5ad1 Stats: 288 lines in 24 files changed: 98 ins; 131 del; 59 mod 8293824: gc/whitebox/TestConcMarkCycleWB.java failed "RuntimeException: assertTrue: expected true, was false" Reviewed-by: iwalulya, tschatzl ------------- PR: https://git.openjdk.org/jdk20/pull/71
