Review Request for 6878481: Add performance counters in the JDK
Mandy.Chung at Sun.COM
Mon Sep 14 01:31:03 UTC 2009
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());
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)
>> which might be cleaer as:
> 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.
More information about the core-libs-dev