RFR (S) 8246274: G1 old gen allocation tracking is not in a separate class

Luo, Ziyi luoziyi at amazon.com
Thu Jun 4 16:15:40 UTC 2020


Hi Thomas,

A third revision is here:
http://cr.openjdk.java.net/~phh/8246274/webrev.02

On 6/4/20, 3:51 AM, Thomas Schatzl wrote:

[..]

>>> - I would prefer if there were separate reset_after_[young_]gc() and a
>>> reset_after_full_gc() methods. Initially I asked myselves why for full
>>> gc that first parameter passed to reset_after_gc() is zero, and only
>>> when looking into the code found that it is ignored anyway. I think the
>>> API of that class isn't that huge yet.
>>
>> Make sense. I refactored it into two methods in rev.01:
>> * reset_after_full_gc()        for full GC
>> * reset_after_incremental_gc() for young and mixed GC
>>

> I would prefer to use "reset_after_young_gc" for the latter - almost all
> existing code uses "young" already to indicate incremental gc (except
> the MBeans support code, but this is _very_ old code).
> 
> The "correct" terms of the two types of young gc would be "young only"
> and "mixed" btw. Even "young only" has for a long time now also
> reclaimed some kind of old gen (humongous) regions.
> I do not expect this to go away in the future but the line blurring even
> more with potentially adding defragmentation work to young only to
> facilitate humongous allocation.

Thanks for the explanation. Fixed in rev.02.

> And as an ultra-minor nit, please remove the "private" visibility
> specifier in the g1OldGenAllocationTracker class as its default.

Done

> As for performance testing I will do that in conjunction with the
> changes to the actual IHOP calculations.

Yep. Besides, I'll add a new gtest for g1OldGenAllocationTracker in the next 
change.

> Do you need a sponsor for this change or has Paul Hohensee offered to do
> that?

Thanks, Paul will sponsor me.

Best,
Ziyi 



More information about the hotspot-gc-dev mailing list