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

[Paul Hohensee]

My bad on the 7003271 webrev.  It's already pushed.  The only one to
review is 6173675.

I replaced initialLongArray with java.util.Arrays.fill.

I took the code for ThreadCpuTimeArray.java from test/java/lang/management/
ThreadCpuTime.java.  The check for DELTA was in that code, so I kept 
it.  Same
with the argument of 200 to goSleep().  I'm assuming that the same logic 
works,
and it appears to do so.  I can, though, change it to be like the code in
ThreadAllocatedMemoryArray.java.

Paul

On 1/9/11 9:17 PM, David Holmes wrote:
> 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
>>