Discussion on JDK-8245511: G1 adaptive IHOP does not account for reclamation of humongous objects by young GC

[Luo, Ziyi]

Hi Thomas,

On 5/27/20, 3:24 AM, Thomas Schatzl wrote:
    
>Hi,
>
>On 26.05.20 20:21, Luo, Ziyi wrote:
>> Hi Thomas
>>
>> Thank you for looking into JDK-8245511. I open this thread to discuss the
>> questions you raised in comments (I’m not an author yet :)).

>There's an easy way to get author quickly and subsequently be much
>faster: instead of trying to get in a huge patch in that takes multiple
>review rounds, each round having a long turnaround time because you do
>not have access to the tools (jira, cr.openjdk), first do two "boring"
>starter bugs (at today's count there are 55 "starter" bugs open in the
>gc component), some of which would probably take you half an hour or
>less each. ;)

>>> If you compare latest jdk15 behavior which does not show this problem here
>> at all like with earlier jdks (you tagged 15 as affected, but I can't
>> reproduce with latest. Might be a local fluke though).
>>
>> You are right about the test result. The probem is not reproducible in
>> jdk-15-ea+24. I look deep into the gc log, I found some anomalies with the
>> humongous regions e.g.,
>>
>> % cat gc-vanilla-15-30353-2020-05-26_17-03-39.log | grep "Humongous regions"
>> [3.256s][info ][gc,heap        ] GC(0) Humongous regions: 228->0
>> [10.209s][info ][gc,heap           ] GC(2) Humongous regions: 78->0
>> ...
>> [20.034s][info ][gc,heap           ] GC(5) Humongous regions: 0->0
>> [20.914s][info ][gc,heap           ] GC(7) Humongous regions: 0->0
>> <No humongous allocations at all according to the gc heap stats>
>> ...
>>
>> I suspect this is because the humongous object size is set the same as the
>> region size in the repro program and somehow jdk15 treats such objects as not
>> humongous (Correct if I were wrong). I adjusted the humongous object size 1.5
>> times of the region size. Now the problem is reproducible in jdk15. I'll
>> update the repro program in JBS.
>
>The dummy allocations made in the program are always humongous, nothing
>changed here (half a region size). However the C2 compiler optimizes
>away these useless allocations. :)
>
>I.e. run with gc+region=trace logging and look for "ALLOC(HUMS" with and
>without some additional code to do something "useful" with them, e.g.
>"System.out.println(newHumongousObject())" or assign it to a static member.
>
>Increasing the humongous object size makes the program work for some
>reason, probably C2 has a threshold for object size it will consider for
>this optimization, but that does not make the test more correct.
>
>Let's keep further discussion in the mailing list since all of us can
>directly participate. I will just post a final reference to this thread
>in JIRA.
>
>Looking forward to your review request. :) (Fwiw, at this time
>reviewing, or at least reviewers okay'ing it, must happen in the mailing
>list as per https://openjdk.java.net/contribute/).

First, my appreciation for explaining the Adaptive IHOP mechanism and the big 
picture in great details in the other thread, especially the 
additional_buffer_bytes part which I was quite confused when reading the 
source code. I see the need for feeding more allocation info to the Adaptive 
IHOP in order to achieve a more accurate estimation of both the long-term 
allocation rate and short-term allocation spike. In reply to two of your 
comments:

> In email: the change imho needs to provide some measure of the spikyness of 
the allocation between gcs
> In JBS: one other improvement could be to put the old gen allocation 
tracking into its own class (e.g. G1OldGenAllocationTracker; which G1Policy 
has an instance of)

I totally agree on tracking the old gen allocation in a separate class which 
allows us to track and feed more allocation info to the Adaptive IHOP. That's 
the reason I haven't publish an RFR for JDK-8245511. The question I raised in 
the other email thread
https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-May/029824.html
shows that not all humongous allocations are properly tracked, which I believe 
can be addressed at the same time while refactoring the old gen tracking into 
a separate class. Without fixing the humongous tracking, my first webrev 
revision is premature for a review. I also agree not to overload Adaptive IHOP 
for tracking the humongous allocation history. Here's my current plan toward 
fixing JDK-8245511:

* I'll create a separate bug ID for refactoring the G1 old gen allocation 
tracking and providing better humongous allocation tracking and submit an RFR 
for that.
* I'll then refactor my current patch and address comments in the JBS for 
JDK-8245511 based on the refactored old gen allocation tracker.

Does the plan make sense to you?

Besides, thanks for explaining how C2 optimized out the dummy humongous 
allocations in JDK15 :)

Best,
Ziyi