RFR: 8191471: Elastic TLABs for G1

Thomas Schatzl thomas.schatzl at oracle.com
Wed May 2 11:06:21 UTC 2018


Hi,

On Wed, 2018-05-02 at 11:28 +0200, Stefan Johansson wrote:
> Thanks for the review Per,
> 
> On 2018-04-24 11:19, Per Liden wrote:
> > Hi Stefan,
> > 
> > On 04/23/2018 02:57 PM, Stefan Johansson wrote:
> > > Hi,
> > > 
> > > Please review these changes to lower the waste in G1 mutator
> > > regions.
> > > 
> > > JBS: https://bugs.openjdk.java.net/browse/JDK-8191471
> > > Webrev: http://cr.openjdk.java.net/~sjohanss/8191471/00/
> > 
> > Looks good overall, just two comments:
> > 
> > src/hotspot/share/gc/shared/collectedHeap.cpp
> > ---------------------------------------------
> > 
> >  374   size_t minimal_tlab_size = 
> > MAX2(ThreadLocalAllocBuffer::compute_min_size(size), MinTLABSize);
> > 
> > Can we move the MAX2-logic into compute_min_size()? It seems to
> > belong in there, rather than here?
> 
> I agree and I changed it. This will however change the logic in 
> compute_size a bit, it didn't take MinTLABSize into consideration 
> before, but now does. I still think the change is good though.
> > 
> > src/hotspot/share/gc/shared/collectedHeap.hpp
> > ---------------------------------------------
> >  133   virtual HeapWord* allocate_new_tlab(size_t min_word_size,
> >  134                                       size_t
> > desired_word_size,
> >  135                                       size_t*
> > actual_word_size);
> > 
> > The argument names now vary between the collectors. I suggest we
> > use the same names in all places and skip the "word" part.
> > Something like: min_size, requested_size, actual_size?
> > 
> 
> Fixed.
> 
> New webrevs:
> Full: http://cr.openjdk.java.net/~sjohanss/8191471/02
> Inc: http://cr.openjdk.java.net/~sjohanss/8191471/01-02
> 

  still good.

Thomas




More information about the hotspot-gc-dev mailing list