RFR(S): 8132721: Add tests which check that heap counters work as expected during Humongous allocations

[Jon Masamitsu]

On 02/02/2016 02:09 PM, Kirill Zhaldbybin wrote:
> Jon,
>
> Thank you for reviewing the fix.
>
> On 02/02/2016 10:53 PM, Jon Masamitsu wrote:
>>
>> Kirill,
>>
>> Would it make sense to add another full GC after line 177?
> I think you are right - it could make the test more stable.
> I also added additional output to separate this full GC from ones that 
> are called after deallocations.
>
> Here are a new webrev: 
> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132721/webrev.06/

Sorry I didn't catch this sooner but at lines 150 and 176

"no GC should happened"

should be

"no GC should happen"

Otherwise, looks good.  I don't need another
webrev.

Jon


>
> Regards, Kirill
>>
>>
>>>   175
>>>   176         System.out.println("Finished allocations - no GC 
>>> should happened before this line");
>>>   177
>>>   178
>>>   179         allocations.stream().forEach(allocation -> {
>>>   180             long usedMemoryBefore = 
>>> memoryCounter.getUsedMemory();
>>>   181             allocation.forgetAllocation();
>>>   182
>>>   183             WHITE_BOX.fullGC();
>>
>> It would remove some uncertainty in the start of the heap  for the
>> first iteration of the loop at 179.
>>
>> Jon
>>
>> On 2/2/2016 7:37 AM, Kirill Zhaldybin wrote:
>>> Dmitry,
>>>
>>> Thank you for reviewing this test.
>>>
>>> Here are a new webrev: 
>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132721/webrev.05/
>>>
>>> Could you please read comments inline?
>>>
>>> Regards, Kirill
>>>
>>> On 02.02.2016 15:25, Dmitry Fazunenko wrote:
>>>> Hi Kirill,
>>>>
>>>> In general the test looks good.
>>>>
>>>> A few minor comments:
>>>>
>>>> 1) copyrights 2015 -> 2016
>>> Fixed
>>>> 2) Would you consider to move
>>>> gcBeans.stream().mapToLong(GarbageCollectorMXBean::getCollectionCount).sum(); 
>>>>
>>>>
>>>> into a separate method. You use this construct twice.
>>> Well, I think twice is still ok and takes less lines that separate 
>>> function.
>>>>
>>>> 3) You print debug info:
>>>>
>>>>              System.out.println("Counter returned that allocation 
>>>> used "
>>>> + (usedMemoryAfter - usedMemoryBefore) + "; "
>>>>                      + "expected allocation size at least " +
>>>> expectedAllocationSize * ALLOCATION_SIZE_TOLERANCE_FACTOR);
>>>>
>>>>   for the sake of simplification debug in case of failure, it's better
>>>> to print separately:
>>>> - allocationSize, usedMemoryBefore, usedMemoryAfter.
>>> Fixed.
>>>
>>>>
>>>> 4)
>>>>
>>>>              if (gcCountNow == gcCountBefore) {
>>>>                  ...
>>>>              } else {
>>>>                  System.out.println("GC happened during allocation so
>>>> the check is skipped");
>>>>              }
>>>>
>>>> In case of an unexpected GC happens on iteration #3, all the following
>>>> iterations will be skipped.
>>>> But you can skip only iteration #3. For that in the 'else' branch you
>>>> need to add
>>>>          gcCountBefore = gcCountNow
>>> Fixed.
>>>
>>>>
>>>> Thanks,
>>>> Dima
>>>>
>>>>
>>>>
>>>> On 01.02.2016 22:04, Kirill Zhaldybin wrote:
>>>>> Dear all,
>>>>>
>>>>> Could you please take a look at this test for JDK-8132721?
>>>>>
>>>>> The test allocates/deallocates humongous objects and checks that
>>>>> memory counters (from Runtime and MemoryMXBean classes) work right.
>>>>>
>>>>> For example it's expected that after allocating HO of size
>>>>> (G1Region/2 + 1) bytes free memory should decrease on full G1Region
>>>>> size.
>>>>>
>>>>> CR:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8132721
>>>>>
>>>>> WebRev:
>>>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132721/webrev.04/
>>>>>
>>>>>
>>>>> Thank you.
>>>>> Regards, Kirill
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160203/0e61a4fc/attachment.htm>