[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-gc-dev
Subject:    Re: RFR: 8202140: TLAB logging is not correct for G1
From:       Thomas Schatzl <thomas.schatzl () oracle ! com>
Date:       2018-04-24 9:32:08
Message-ID: 1524562328.2367.4.camel () oracle ! com
[Download RAW message or body]

Hi Stefan,

On Tue, 2018-04-24 at 10:32 +0200, Stefan Johansson wrote:
> Hi Thomas,
> 
> On 2018-04-23 15:42, Thomas Schatzl wrote:
> > Hi,
> > 
> > On Mon, 2018-04-23 at 14:57 +0200, Stefan Johansson wrote:
> > > Hi,
> > > 
> > > Please review this small change to make TLAB logging more
> > > correct,
> > > especially for collectors using regions.
> > > 
> > > JBS: https://bugs.openjdk.java.net/browse/JDK-8202140
> > > Webrev: http://cr.openjdk.java.net/~sjohanss/8202140/00/
> > > 
> > > Summary
> > > The current TLAB logging is not completely correct for G1 and
> > > other
> > > collectors using regions. The logging in
> > > ThreadLocalAllocBuffer::print_stats() expects that the allocated
> > > size
> > > is: size_t alloc = _number_of_refills * _desired_size;
> > > 
> > > For each region in G1 the last TLAB might be smaller then
> > > _desired_size and therefor we need to add a separate value to
> > > account
> > > the actual allocated size.
> > 
> >    looks good to me.
> > 
> > Maybe please fix the indentation of
> > ThreadLocalAllocBuffer::initialize_statistics().
> > 
> 
> Thanks for the review, I will add this before pushing:
> ===
> diff --git a/src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp 
> b/src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp
> --- a/src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp
> +++ b/src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp
> @@ -157,12 +157,12 @@
>   }
> 
>   void ThreadLocalAllocBuffer::initialize_statistics() {
> -    _number_of_refills = 0;
> -    _fast_refill_waste = 0;
> -    _slow_refill_waste = 0;
> -    _gc_waste          = 0;
> -    _slow_allocations  = 0;
> -    _allocated_size    = 0;
> +  _number_of_refills = 0;
> +  _fast_refill_waste = 0;
> +  _slow_refill_waste = 0;
> +  _gc_waste          = 0;
> +  _slow_allocations  = 0;
> +  _allocated_size    = 0;
>   }
> 
>   void ThreadLocalAllocBuffer::fill(HeapWord* start,
> ===

  thanks, looks good.

Thomas

[prev in list] [next in list] [prev in thread] [next in thread]