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

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Thu Feb 4 15:07:23 UTC 2016


Jon,

Thank you for review!

Regards, Kirill

On 04.02.2016 01:30, Jon Masamitsu wrote:
>
>
> 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
>>>>>>
>>>>>
>>>>
>>>
>>
>




More information about the hotspot-gc-dev mailing list