Pls (re)review 6173675/7003271: per-thread memory allocation measurement

David Holmes David.Holmes at oracle.com
Sun Jan 9 18:17:39 PST 2011


Hi Paul,

Partial re-review :)

Paul Hohensee said the following on 01/10/11 11:10:
> Thanks to all who reviewed the previous webrev.  Revised versions are here
> 
> http://cr.openjdk.java.net/~phh/6173675/webrev.02/

In ThreadImpl.java you could replace:

     long[] times = initialLongArray(length, -1);

with

     long[] times = java.util.Arrays.fill(new long[length], -1);

and delete initialLongArray.


In test/com/sun/management/ThreadMXBean/ThreadCpuTimeArray.java

I don't understand the test logic. Your tests threads all do:

doit();
while (!done) wait
doit();

but the main thread doesn't check any times etc after setting done=true, 
so why have the test threads do the second call to doit? For that matter 
why even bother with doit()? It only delays the threads in getting to 
the wait(), which is what the main thread itself waits for.

  111         goSleep(200);

What does 200 represent? What is it that you are actually waiting for? 
Is 200 long enough on a slow uniprocessor?

  141             if ((times[i] + DELTA) < newTime) {
  142                 throw new RuntimeException("TEST FAILED: " +

Again what does DELTA represent, how can you be sure the times for all 
threads will have advanced this much?


> http://cr.openjdk.java.net/~phh/7003271/webrev.02/

All files listed have zero changes ???

Cheers,
David
-----

> The interesting changes are in the library side for 6173675.  In 
> particular,
> I've added 3 unit tests.
> 
> Thanks for any review,
> 
> Paul
> 


More information about the serviceability-dev mailing list