RFR (M): 8212657: Implementation of JDK-8204089 Promptly Return Unused Committed Memory from G1 [Was: RFR (M): 8212657: Implementation of JDK-8204089 Timely Reduce Unused Committed Memory]

[sangheon.kim at oracle.com]

Hi Thomas,

On 12/3/18 6:30 AM, Thomas Schatzl wrote:
> Hi Sangheon,
>
> On Sun, 2018-12-02 at 14:34 -0800, sangheon.kim at oracle.com wrote:
>> Hi Thomas,
>>
>> On 11/12/18 1:39 AM, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>
> [...]
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8212657
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8212657/webrev/index.html
>> Looks good.
>> I have just minor nits:
>>
>> ---------------
>>    src/hotspot/share/gc/shared/gcCause.cpp
>>    src/hotspot/share/gc/shared/gcCause.hpp
>>    src/hotspot/share/gc/shared/vmGCOperations.cpp
>> - Copyright update
>>
>> ---------------
>> test/hotspot/jtreg/gc/g1/ihop/TestIHOPErgo.java
>> -        runTest(64, 50, true);
>> +        runTest(64, 100, true);
>> - Why we need this change? This feature is basically disabled.
> The only issue I remember is that with an allocation delay of 50 we run
> into quite a lot of evacuation failures in this test. But that is
> unrelated to this issue. Removed.
>
>> ---------------
>>     73             System.out.println("Skipped. Initial heap size is
>> too
>> close to max heap size.");
>> - The log message slightly makes me confused. The test is skipped
>> because we don't have enough free/available memory to run the test
>> without triggering GC. 'Initial heap size' was looking as
>> 'InitialHeapSize' at first. :)
> All fixed.
>
> http://cr.openjdk.java.net/~tschatzl/8212657/webrev.1_to_2/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8212657/webrev.2/ (full)
Webrev.2 looks good.

Thanks,
Sangheon


>
>    thanks for your review.
>
> Thanks,
>    Thomas
>
>