RFR: 7109087: gc/7072527/TestFullGCCount.java fails when GC is set in command-line

[Kevin Walls]

Thanks - I will!


On 18/04/13 15:51, Mikael Gerdin wrote:
> Kevin,
>
> On 2013-04-18 16:15, Kevin Walls wrote:
>>
>> OK, yes great we can even initialise the ArrayList in its constructor to
>> help reduce that risk.  Updated in the same location as below.
>
> Great, ship it!
>
> /Mikael
>>
>> Thanks!
>> Kevin
>>
>>
>>
>> On 18/04/13 14:32, Mikael Gerdin wrote:
>>> Kevin,
>>>
>>> On 04/17/2013 05:53 PM, Kevin Walls wrote:
>>>> On 17/04/13 13:05, Mikael Gerdin wrote:
>>>>> Kevin,
>>>>>
>>>>> On 2013-04-10 16:05, Kevin Walls wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I'd like a review of this testcase change.  It fails as it has a GC
>>>>>> option set in the jtreg tag which may conflict with what jtreg 
>>>>>> sends,
>>>>>> and fail to run.
>>>>>>
>>>>>> Although the test was created for a CMS problem, double-counting of
>>>>>> full
>>>>>> collections, here it is adapted to check all collectors:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~kevinw/7109087/webrev/
>>>>>
>>>>> Since you know the number of iterations I think it would be better to
>>>>> use an ArrayList and use list.ensureCapacity(20) to avoid allocations
>>>>> in addCollectionCount.
>>>>>
>>>>> You could also make the GarbageCollectorMXBean list available from a
>>>>> static variable since I'm not sure what happens in that call path.
>>>>>
>>>>> Here you access theseCounts as a List instead of a List<Long>, maybe
>>>>> the "counts" map should be a Map<String, List<Long>>?
>>>>>
>>>>> +            for (int i = 0; i < iterations - 1; i++) {
>>>>> +                List theseCounts = counts.get(collector);
>>>>> +                long a = (Long) theseCounts.get(i);
>>>>>
>>>>> /Mikael
>>>>>
>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> Kevin
>>>>
>>>>
>>>> Hi Mikael, thanks -
>>>>
>>>> ensureCapacity: not sure it stops the addCollectionCount() method
>>>> changing/adding to the list?  We could verify the list is of the right
>>>> length, although I don't think it's that important?...
>>>
>>> I meant that ensureCapacity() on ArrayList will make sure that we
>>> won't need to reallocate the backing array when adding elements.
>>> LinkedList on the other hand will allocate a new list node every time
>>> you add an element to the list.
>>>
>>> We can't be 100% sure that we won't trigger a spurious GC when running
>>> the test but if we avoid allocating we can reduce the risk somewhat.
>>>
>>> /Mikael
>>>
>>>> addCollectionCount adds one thing to the List for each collector each
>>>> time it's called...  I don't think we can guarantee that collectors
>>>> aren't getting invoked at any time, but we can make it unlikely, like
>>>> the next point you make:
>>>>
>>>> Yes, don't really need to call  into the ManagementFactory each
>>>> iteration, and can reduce it to a single call to
>>>> ManagementFactory.getGarbageCollectorMXBeans();  We don't need the
>>>> List<MemoryPoolMXBean> at all.
>>>>
>>>> Yes, those casts to Long can be eradicated.
>>>>
>>>> Also there was an unused "count" variable in main(), removed it.. 8-)
>>>>
>>>> And I should clone from hotspot-gc for this change.
>>>>
>>>> New webrev: http://cr.openjdk.java.net/~kevinw/7109087/webrev.02/
>>>>
>>>> Thanks
>>>> Kevin
>>>>
>>>
>>