RFR: 8057536: Refactor G1 to allow context specific allocations
Mikael Gerdin
mikael.gerdin at oracle.com
Fri Sep 5 07:15:38 UTC 2014
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
>
> 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
More information about the hotspot-gc-dev
mailing list