RFR(S) 8212127: Cleanup TLAB fast refill statistics, perf counters and etc.

Per Liden per.liden at oracle.com
Thu Feb 7 09:20:02 UTC 2019


Hi,

On 2/6/19 3:48 PM, Aleksey Shipilev wrote:
> On 2/6/19 3:34 PM, zgu at redhat.com wrote:
>>> *) In src/hotspot/share/gc/shared/threadLocalAllocBuffer.hpp, why
>>> this whole thing is removed? I
>>> would expect "just" the rename of slow_refill_waste() to
>>> refill_waste() and removing
>>> fast_refill_waste() here, leaving everything else untouched.
>>>
>>>   105   // statistics
>>>   106
>>>   107   int number_of_refills() const { return _number_of_refills; }
>>>   108   int fast_refill_waste() const { return _fast_refill_waste; }
>>>   109   int slow_refill_waste() const { return _slow_refill_waste; }
>>>   110   int gc_waste() const          { return _gc_waste; }
>>>   111   int slow_allocations() const  { return _slow_allocations; }
>>>   112
>>
>> They are dead code. Do they worth for another cleanup RFE for the
>> trivial cleanup?
> 
> Okay. I thought the patch was about fast refills only. But indeed, we can clean these unused
> definitions in the same patch. On one hand, removing trivial unused getters would require us to add
> them back when they are needed. On the other hand, it seems that TLAB does every statistic
> internally, and we should instead force callers to use higher-level TLAB methods to access and
> interpret them.
> 
> Code looks good! I think we still need Serviceability to acknowledge this is okay to do.

Thanks for cleaning this up. GC changes look good. Just one minor thing, 
please align the assignment here:

@@ -147,8 +145,7 @@

  void ThreadLocalAllocBuffer::reset_statistics() {
    _number_of_refills = 0;
-  _fast_refill_waste = 0;
-  _slow_refill_waste = 0;
+  _refill_waste = 0;
    _gc_waste          = 0;
    _slow_allocations  = 0;
    _allocated_size    = 0;


I agree that Serviceability should ack the jstat change.

cheers,
Per


More information about the hotspot-gc-dev mailing list