RFR: 8293824: gc/whitebox/TestConcMarkCycleWB.java failed "RuntimeException: assertTrue: expected true, was false" [v2]
Ivan Walulya
iwalulya at openjdk.org
Tue Dec 6 19:40:07 UTC 2022
On Tue, 6 Dec 2022 12:34:47 GMT, Kim Barrett <kbarrett at 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 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
>> requests 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 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.
>>
>> Testing:
>> mach5 tier1-6
>
> Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits:
>
> - use new whitebox api for running conc gc
> - Merge branch 'master' into test-concmark
> - copyrights
> - remove WB.g1StartConcMarkCyle
> - update tests
> - add utility GC breakpoint functions
> - add G1ConcurrentMark::completed_mark_cycles() and whitebox access
Lgtm!
Minor comments:
test/hotspot/jtreg/gc/g1/humongousObjects/objectGraphTest/GCTokens.java line 36:
> 34: public static final String WB_INITIATED_YOUNG_GC = "Young (Normal) (WhiteBox Initiated Young GC)";
> 35: public static final String WB_INITIATED_MIXED_GC = "Young (Mixed) (WhiteBox Initiated Young GC)";
> 36: public static final String WB_INITIATED_CMC = "WhiteBox Initiated Run to Breakpoint";
`WB_INITIATED_CMC = "WhiteBox Initiated Run to Concurrent GC Breakpoint"; ` as we indicate below that `String CMC = "Concurrent Mark)"`
test/hotspot/jtreg/gc/stress/TestStressRSetCoarsening.java line 296:
> 294: // Number of references went down.
> 295: // Need to provoke recalculation of RSet.
> 296: WB.g1RunConcurrentGC(false);
probably use `WB.g1RunConcurrentGC();` overload for consistency with other calls
-------------
Marked as reviewed by iwalulya (Reviewer).
PR: https://git.openjdk.org/jdk/pull/11435
More information about the hotspot-dev
mailing list