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