RFR: 8057536: Refactor G1 to allow context specific allocations
Stefan Johansson
stefan.johansson at oracle.com
Thu Sep 4 19:01:11 UTC 2014
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/
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
More information about the hotspot-gc-dev
mailing list