Review Request for 6878481: Add performance counters in the JDK
Alan Bateman
Alan.Bateman at Sun.COM
Thu Sep 3 08:54:55 UTC 2009
Mandy Chung wrote:
> This is related to 6857194: Add hotspot new perf counters to aid class
> loading performance measurement.
>
> It's useful to add performance counters in the library code so that
> perf data from the JDK and VM can be collected and output in a unified
> way (written in the jvmstat shared memory buffer). I add a simple
> sun.misc.PerfCounter class to maintain the list of perf counters for
> the library to use. This fix only instruments the class loading and
> jar/zip to collect simple basic metrics. Additional perf counters
> will be added in the future.
>
> Webrev:
> http://cr.openjdk.java.net/~mchung/6878481/webrev.00/
>
> Thanks
> Mandy
It's good to see the use of the instrumentation buffer extended to the
libraries.
I'm not sure how sun.misc.Perf behaves when running with
-XX:-UsePerfData. You've probably checked this anyway, but just in case...
It would be good to include a comment to define the meaning of each of
the counters. For example, in ClassLoader.loadClass, it's not clear if
you mean to include the time to check if the class has already been
loaded (just on that one, if the lookup is not meant to be included then
it would reduce the calls to nanoTimes a bit).
Is it necessary to have separate counters for zip and JAR? If I'm
reading this correctly, then zipFiles and jarFiles will both be
incremented when a JAR file is opened - is that right?
I agree with RĂ©mi's comment that perf, name, and lb can be final.
Would addElapsedTime(start) be better named as addElapsedTimeFrom(start)
to make it clear that it doesn't add "start" to the elapsed time, but
rather the elapsed time from the given starting time?
This might sound crazy, but do the updates need to be synchronized? I
don't think there is any synchronization in the VM. I agree with David's
concern about the overhead of the calls to nanoTimes (which I'm sure you
are measuring) but for the synchronization there are places where both a
counter and an elapsed time are updated.
-Alan.
More information about the core-libs-dev
mailing list