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

[Per Liden]

Hi,

On 2/6/19 3:48 PM, Aleksey Shipilev wrote:
On 2/6/19 3:34 PM, z...@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