Review Request for 6878481: Add performance counters in the JDK
Andrew John Hughes
gnu_andrew at member.fsf.org
Thu Sep 3 14:43:23 UTC 2009
2009/9/3 Rémi Forax <forax at univ-mlv.fr>:
> Le 03/09/2009 10:54, Alan Bateman a écrit :
>> 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 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.
> Hi Alan,
> Classloading is now done in parallele, zip and jar can be loaded by
> different threads.
> "for the synchronization there are places where both a counter and an
> elapsed time are updated"
> => the problem is that these synchronisations are done on two different
> I don't see how to remove them easily.
> Furthermore I think that all synchronized can not be easily replaced by some
> atomics operations
> (unsafe.put* or unsafe.compareAndSwap) because long in a ByteBuffer aren't
> Perhaps with the Doug Lea's Fences (scheduled to be introduced in jdk7) but
> I'm not sure.
The use of synchronized at present seems flawed as only the set
methods are protected and not the get. So lb.get could be called
while in an lb.put call. I don't see the reason for using a
LongBuffer either, as only index 0 is ever used. Why not use an
What is the purpose of d3dAvailable? I don't know this piece of code
so I'm not sure what it's counting, but it will presumably be zero
forever on non-Windows systems.
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and the OpenJDK
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
More information about the core-libs-dev