[14] RFR (S): 8237079: gc/g1/mixedgc/TestLogging.java fails with "Pause Young (Mixed) (G1 Evacuation Pause) not found"

[Leo Korinth]

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.

/Leo


> 
> Thanks,
>    Thomas
>