RFR (XS): 8232951: TestG1ParallelPhases.java fails with phase NonYoungFreeCSet not found

[Thomas Schatzl]

Hi,

On 28.10.19 11:40, Leo Korinth wrote:
> Hi.
> 
> Just want to add some information, because I think it will fail again.
> 
> The buggy test case is written by me and the provoke mixed gc part is 
> copied mostly either from TestOldGenCollectionUsage or TestLogging (as 
> it is hard to share this code due to JTREG). However when I did "copy" 
> the code I also did try to improve the code, this could be the reason 
> for this failure. I did at least two "improvements" in that I removed 
> magic constants when allocating the 20k arrays and instead calculated 
> how many I would need; this made the algorithm allocate ~2M instead of 
> ~3M which could be a problem although to my understanding it should not 
> be. Another change I made is that I will not provoke a gc by allocating 
> until out-of-memory. The original code seems to try to provoke a gc by 
> starting concurrent marks and young gc, but kind of fail-safes with the 
> code after the comment // allocate more objects to provoke GC. Having 
> this code I guess would fix the problem with the test case, but on the 
> other hand, we would not know why the youngGC() after concurrent mark 
> does not provoke a mixed gc (I guess it should, but correct me if this 
> is false).

I do not think either change makes a difference.

> 
> I have talked to Thomas off-list, and I think AlwaysTenure is not the 
> solution to the problem we have. I think adding the debug options is 
> great and should be done, and AlwaysTenure seems better than 
> MaxTenuringThreshold=1 but we should expect the test case to continue to 
> fail in the future.
> 
> If you go by adding AlwaysTenure instead of MaxTenuringThreshold=1, 
> please also remove one getWhiteBox().youngGC() in allocateOldObjects so 
> that we do not leave "magic" lines in the test case. Also update the 
> comment to // Do *one* young collections...
> and there is another "-XX:MaxTenuringThreshold=1" that needs to be 
> updated. I need no webrev for these changes.

Updated in place; also fixed Kim's comment about line length.

http://cr.openjdk.java.net/~tschatzl/8232951/webrev/

> 
> I am sorry that my "improvements" probably caused this failure, though 
> just having heaps of code and not understanding why, is probably worse 
> in the long run --- at least that is my thinking.

The question I have is whether I can push these changes under this CR 
(and if it occurs again we at least have a log to look at) or use 
another CR for it?

Thanks,
   Thomas