RFR (S): 8237079: gc/g1/mixedgc/TestLogging.java fails with "Pause Young (Mixed) (G1 Evacuation Pause) not found"
leo.korinth at oracle.com
Wed Jan 22 16:02:06 UTC 2020
On 22/01/2020 11:46, Thomas Schatzl wrote:
> Hi all,
> On 17.01.20 16:07, Leo Korinth wrote:
>> Hi Thomas,
>> This is not a review. This code is basically the same code as is
> I took it as one anyway ;)
>> duplicated at least three times in the test code. One of the
>> duplications you can blame me for, *sorry*. I believe it should be
>> moved to a common library method. I also believe the last fix you did
>> in TestG1ParallelPhases.java makes that version look cleaner than what
>> you propose here (it does not need the last allocation loop at all).
>> How about using the TestG1ParallelPhases.java version for all three
>> test cases? If not, do the third version in TestOldGenCollectionUsage
>> really work???
> Here's a webrev incorporating these suggestions to unify the code:
> http://cr.openjdk.java.net/~tschatzl/8237079/webrev.1 (full)
> There is no point to provide a diff webrev here as the whole change has
> been redone.
> - factor out and use a MixedGCProvoker class in all three of those tests.
> - some changes in the various tests to align their option a bit more
> and stabilize them
> - TestOldCollectionUsage assumed that there were no previous old gen
> allocations, actually it failed if there were. Since we can't guarantee
> that, loosened the condition to require update of the mixed gc usage only.
> - for gc/g1/mixedgc/Testlogging.java removed the need to match the
> whole log message including the "G1 Evacuation Pause" gc cause message.
> It did not seem to be point of the test to check that the mixed gc has
> been caused "naturally" by eden exhaustion or via whitebox.
I think this looks *really* good. Also, thanks for taking the extra time
to make it shared and reusable code.
More information about the hotspot-gc-dev