RFR (M): 8073052: Rename and clean up the allocation manager hierarchy in g1Allocator.?pp
Thomas Schatzl
thomas.schatzl at oracle.com
Tue Aug 4 07:48:59 UTC 2015
Hi,
On Mon, 2015-08-03 at 18:43 -0400, Kim Barrett wrote:
> On Jul 29, 2015, at 3:51 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> >
> > On Tue, 2015-07-28 at 14:42 +0200, Mikael Gerdin wrote:
> >> 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)
>
> ------------------------------------------------------------------------------
> src/share/vm/gc/g1/g1Allocator.cpp
> 186 size_t gclab_word_size = _allocator->heap()->desired_plab_sz(dest);
>
> [here and in other places]
> I was expecting to see _g1h->desired_plab_sz(dest) here, in response
> to Mikael's suggestion, but I see you took his "Another option"
> instead. Any reason for that, rather than giving the G1PLABAllocator
> direct access to the heap object?
>
PLABAllocator does not have a local reference to G1CollectedHeap. So I
could have either added one, or used the one from the G1Allocator.
I somewhat tend to dislike the thousands of local copies of
G1CollectedHeap for no reason.
If you strongly prefer making another copy, I can change this likewise.
Thanks for the review,
Thomas
More information about the hotspot-gc-dev
mailing list