RFR (M): 8073052: Rename and clean up the allocation manager hierarchy in g1Allocator.?pp

Jon Masamitsu jon.masamitsu at oracle.com
Tue Aug 4 18:30:55 UTC 2015


Thomas,

I reviewed webrev.4 and it looks good.

Jon


On 07/29/2015 02:04 AM, Mikael Gerdin wrote:
> Thomas,
>
> On 2015-07-29 09:51, Thomas Schatzl wrote:
>> Hi Mikael,
>>
>>    thanks for the review.
>>
>> On Tue, 2015-07-28 at 14:42 +0200, Mikael Gerdin wrote:
>>> Hi Thomas,
>>>
>>> On 2015-07-28 10:46, Thomas Schatzl wrote:
>>>> Hi all,
>>> [...]
>>>> Finally, the change aligns the naming of G1ParGCAllocator with other
>>>> classes that use "PLAB" in their names instead of "ParGC".
>>>>
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8073052
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~tschatzl/8073052/webrev.3/
>>>
>>> Overall a nice cleanup!
>>>
>>> g1Allocator.cpp:
>>> In unsafe_max_tlab_alloc you get the heap via G1H::heap() instead of 
>>> _g1h.
>>
>> Fixed.
>>
>>> G1PLABAllocator seems to look up the G1H::heap() fairly often,
>>> did you consider passing on the G1H* to the G1PLABAllocator to reduce
>>> the clutter from accessing through the static getter in multiple
>>> locations? Another option could be to expose the G1H* from G1Allocator,
>>> yielding _allocator->heap()->...
>>> instead of _g1h->...
>>
>> Fixed.
>>
>> New webrevs:
>> http://cr.openjdk.java.net/~tschatzl/8073052/webrev.4/ (full)
>> http://cr.openjdk.java.net/~tschatzl/8073052/webrev.3_to_4/ (diff)
>
> Looks good to me.
> /Mikael
>
>>
>> Thanks,
>>    Thomas
>>
>>
>



More information about the hotspot-gc-dev mailing list