Review Request for 6878481: Add performance counters in the JDK
Mandy.Chung at Sun.COM
Thu Sep 3 18:02:02 UTC 2009
Alan Bateman wrote:
> 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.
> It's good to see the use of the instrumentation buffer extended to the
> I'm not sure how sun.misc.Perf behaves when running with
> -XX:-UsePerfData. You've probably checked this anyway, but just in
Adding -XX:-UsePerfData option works fine but I am yet to understand the
logics since this should be different than the case with a small
PerfDataMemorySize where the perf counters are still created but in the
C heap. I'm looking into the hotspot perfdata implementation further.
> 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).
As David points out the high number of calls to System.nanoTime, I
should refine the fix not to include the lookup time. I'll update the
fix and also describes the meaning of each counter.
> 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?
That's right, both zipFiles and jarFiles counters are incremented.
> 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.
Yes, I ran the server benchmarks on Windows XP that shows negligible
overhead. I have to rerun the solaris-i586 measurement as I got some
high standard deviation. I'm composing my reply to David's email.
With the synchronization, we might miss some updates. It's a tradeoff
between correctness and performance overhead (I have to find out the
reasons why it was decided not to have synchronization in the hotspot
implementation). Is the counter update a fast operation - updating the
direct buffer? I think so and thus this should not cause high lock
contention on the PerfCounter.
More information about the core-libs-dev