RFR: 8057536: Refactor G1 to allow context specific allocations

[Stefan Johansson]

On 2014-09-05 09:15, Mikael Gerdin wrote:
> Hi Stefan,
>
>
> On Thursday 04 September 2014 21.01.11 Stefan Johansson wrote:
>> Hi Mikael,
>>
>> On 2014-09-04 17:04, Mikael Gerdin wrote:
>>> Hi Stefan,
>>>
>>> On Thursday 04 September 2014 14.45.30 Stefan Johansson wrote:
>>>> Hi,
>>>>
>>>> Please review these changes for:
>>>> https://bugs.openjdk.java.net/browse/JDK-8057536
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~sjohanss/8057536/webrev.00/
>>> g1Allocator.hpp:
>>>
>>> reuse_retaind_... => reuse_retained_...
>>>
>>> G1ParGCAllocator::allocate does not have the SurvivorAlignmentInBytes
>>> change when it was broken out from G1ParScanThreadState.
>> Fixed.
>>
>>> g1Allocator.cpp:
>>>
>>> reuse_retaind_... => reuse_retained_...
>>> two spaces after return type of retire_alloc_buffers
>>> align parameters to flush_stats_and_retire
>> Fixed.
>>
>>> g1CollectedHeap.hpp:
>>>
>>> align the parameters to "attempt_allocation_at_safepoint",
>>> "survivor_attempt_allocation", "old_attempt_allocation"
>> Fixed.
>>
>>> g1CollectedHeap.cpp:
>>>
>>> You didn't move the methods of Mutator/SurvivorGC/OldGC alloc region to
>>> g1Allocator.cpp
>> Realized that those were missed when moving stuff before, and Bengt
>> pointed out that they probably should move to G1AllocRegion.hpp as well,
>> so that has been done.
>>
>>> align the parameters at the call to "attempt_allocation"
>>> align the parameters to "attempt_allocation_at_safepoint"
>> Fixed.
>>
>>> g1CollectedHeap.inline.hpp:
>>>
>>> align the parameters at the call to "attempt_allocation"
>>>
>>> align the parameters to "survivor_attempt_allocation",
>>> "old_attempt_allocation"
>> Fixed.
>>
>>> heapRegion.cpp:
>>>
>>> HeapRegion::print_on is designed to produce short output. Can you shorten
>>> the output? Either by disabling the printout if contexts aren't used or
>>> disable it for context-0 regions. Or have a format like "C%4u".
>> Change to "AC%4u".
>>
>>> For most of the style changes I'm ok with a follow-up change to address
>>> them. The rest of the change looks ok to me.
>> As you see I've addressed most of them, here is an updated webrev as
>> well as a diff between the two:
>> http://cr.openjdk.java.net/~sjohanss/8057536/webrev.01/
>> http://cr.openjdk.java.net/~sjohanss/8057536/webrev.00-01/
> Looks good.
> /Mikael
Many big thanks for the review Mikael!

Stefan
>> I also had to do a small addition in vm_operations.hpp, which was
>> forgotten in the previous webrev.
>>
>> Thanks for doing a through review,
>> Stefan
>>
>>> I'm not a 8u Reviewer.
>>>
>>> /Mikael
>>>
>>>> Summary:
>>>> These changes are made to allow G1 to do context specific allocation. As
>>>> part of this a G1Allocator class has be refactored out of
>>>> G1CollectedHeap which can be extended to implement the specific context
>>>> code. Currently only the G1DefaultAllocator is implemented, this
>>>> allocator only makes use of one context to have the same behavior as
>>>> prior to this enhancement.
>>>>
>>>> Testing:
>>>> * JPRT
>>>> * Manual verification
>>>> * Local ute runs of vm.quick.testlist
>>>>
>>>> Thanks,
>>>> Stefan
> H