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]

[Stefan Johansson]

On 2018-12-03 15:30, 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)
Still good,
StefanJ
> 
>    thanks for your review.
> 
> Thanks,
>    Thomas
> 
>