Review Request for 6878481: Add performance counters in the JDK

Mandy Chung Mandy.Chung at Sun.COM
Mon Sep 14 01:31:03 UTC 2009


Alan, David,

Thanks for the review.  Comments inlined below.

Alan Bateman wrote:
> Minor nit but I assume you can initialize perf as:
>  private static final Perf perf =
>     AccessController.doPrivileged(new Perf.GetPerfAction());

Fixed.

David Holmes wrote:
> Just to add 2c to Alan's method naming comments:
>
> Alan Bateman said the following on 09/13/09 18:07:
>> Method naming is hard (and often subjective) but there are updates 
>> like this:
>>  PerfCounter.getParentDelegationTime.inc(t1 - t0);
>> which might be easier to read as:
>>  PerfCounter.getParentDelegationCounter().addTime(t1 - t0) 
>
>
> There's always a problem with method naming when a thing can be both a 
> counter and a time-tracker. I would agree to use context specific 
> methods even at the expense of some redundancy in the public API eg:
>
> addTime(long interval)
> addElapsedTimeFrom(long startTime)
>
>> Also:
>>  PerfCounter.getZipFileCount().inc();
>> which might be cleaer as:
>>  PerfCounter.getZipFileCounter().increment();
>
> I'd also suggest (in a convention used elsewhere eg 
> java.util.concurrent.atomic.*) that you reserve increment() for 
> addition of 1 and use add(long val) for the general case.
>
I agree that addTime(long interval), increment(), and add(long val) are 
better method names.  I'll make the change.

As for PerfCounter.getParentDelegationTime() vs 
getParentDelegationCounter(), I would prefer to keep 
getParentDelegationTime as the method name so that it's clear that this 
counter is a time tracker.  Similarly, it's clear that getZipFileCount() 
is a counter to keep track of zip file count.

Thanks
Mandy




More information about the core-libs-dev mailing list