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