RFR: 8071462: Remove G1ParGCAllocator::alloc_buffer_waste

[Stefan Johansson]

Hi Michail,

On 2015-04-17 19:46, Michail Chernov wrote:
> Sorry, typos have been fixed in the message:
>
> Webrev: http://cr.openjdk.java.net/~dpochepk/~mchernov/8071462/webrev.03/
>
I agree with Thomas comments about moving the implementation to the 
cpp-file and adding the missing header. I also found a small issue with 
the updated assert message in plab.cpp:
101                    "_allocated: "SIZE_FORMAT", "
102                    "_wasted: "SIZE_FORMAT", "
103                    "_unused: "SIZE_FORMAT", ",
104                    "_undo_wasted: "SIZE_FORMAT,
105                    _allocated, _wasted, _unused, _undo_wasted));
106

On line 103, you need to remove the trailing comma (',') to make the 
format string correct.

Otherwise change looks ok.

Thanks,
Stefan

> Methods were renamed, removed extra comma, class G1PLABWasteStat was 
> removed, now G1PLAB::waste(size_t& wasted, size_t& undo_wasted) gets 2 
> references. Also I merged my changes with new changes in repo because 
> ParGCAllocBuffer and G1ParGCAllocBuffer were renamed to PLAB and 
> G1PLAB and patch does not apply clear during merging.
>
> Thanks,
> Michail
>
> On 17.04.2015 20:30, Michail Chernov wrote:
>> Hi Thomas,
>>
>> Thanks for advice!
>>
>> http://cr.openjdk.java.net/~dpochepk/~mchernov/8071462/webrev.03/
>>
>> Methods were renamed, removed extra comma, class G1PLABWasteStat was 
>> removed, now G1PLAB::waste(size_t& wasted, size_t& undo_wasted) gets 
>> 2 references. Also I merged my changes with new changes in repo 
>> because ParGCAllocBuffer and G1ParGCAllocBuffer were renamed to PLAB 
>> and G1PLAB and patch does not apply clear during merging.
>>
>> Thanks,
>> Michail
>>
>> On 17.04.2015 14:38, Thomas Schatzl wrote:
>>> Hi,
>>>
>>> On Fri, 2015-04-17 at 14:23 +0300, Michail Chernov wrote:
>>>> Hi Thomas,
>>>>
>>>> Thanks for looking!
>>>>
>>>> On 17.04.2015 14:08, Thomas Schatzl wrote:
>>>>> Hi Michail,
>>>>>
>>>>> On Wed, 2015-04-15 at 21:42 +0300, Michail Chernov wrote:
>>>>>> Hi Thomas,
>>>>>>
>>>>>> I tried to make all what you said.
>>>>>> Webrev: 
>>>>>> http://cr.openjdk.java.net/~dpochepk/~mchernov/8071462/webrev.02/
>>>>>>
>>>>>> ParGCAllocBuffer::add_undo_waste and
>>>>>> ParGCAllocBuffer::undo_last_allocation were made as protected 
>>>>>> members.
>>>>>>
>>>>>> ParGCAllocBuffer::undo_allocation now does the same as
>>>>>> ParScanThreadState::undo_alloc_in_to_space and
>>>>>> G1ParGCAllocator::undo_allocation
>>>>>>
>>>>>> G1ParGCAllocator has virtual G1PLABWasteStat wasted() = 0; method 
>>>>>> which
>>>>>> must be implemented in all heirs.
>>>>>>
>>>>>> Introduced new class G1PLABWasteStat (g1Allocator.hpp) that 
>>>>>> retrieves
>>>>>> wasted and undo wasted values from allocation buffers. If new 
>>>>>> class is
>>>>>> not acceptable there, I can add 2 virtual methods to 
>>>>>> G1ParGCAllocator
>>>>>> for retrieving wasted and undo wasted values.
>>>>>     some comments:
>>>>>
>>>>> - Introduction of G1PLABWasteStat fine with me, although I would 
>>>>> not add
>>>>> an extra type just for two values to be returned.
>>>> Should I create 2 separate virtual method to get wasted and undo 
>>>> wasted
>>>> values? Or I can use Pair template which is from utilities/pair.hpp.
>>> Just two parameters which are passed by reference (we typically use
>>> pointers but I do not mind either) would be fine, that's why I said it
>>> would be preferable to me in this situation :) The struct isn't that
>>> much used (actually only in this place) and seems to add a lot of
>>> unnecessary boiler plate code just for transferring that information in
>>> this single place.
>>>
>>> Thanks,
>>>    Thomas
>>>
>>>
>>>
>>>
>>
>>
>>
>