On 11/5/2013 5:07 PM, David Holmes wrote:
Thanks Mandy.
One minor nit:
Having goSleep be an instance method on ThreadStateController makes
the usage look weird:
thread.goSleep(10); // actually makes current thread go sleep
Good catch. I changed it to a static pause(long ms) method. Changeset
pushed.
thanks for the review.
Mandy
I suggest making it static and use static import to shorten the call
site :)
If you do change that, no need for re-review.
David
On 6/11/2013 7:58 AM, Mandy Chung wrote:
On 11/4/2013 8:42 PM, David Holmes wrote:
This looks good to me. One nit that caused me some confusion - it took
me a while to realize that in transitionTo eg:
221 public void transitionTo(Thread.State tstate) throws
InterruptedException {
222 switch (tstate) {
223 case RUNNABLE:
224 nextState(RUNNABLE);
225 break;
The case value, eg RUNNABLE, and the arg to nextState, eg RUNNABLE,
are two completely different values! Can I suggest using S_xxx for the
int states (why not an enum?).
Good suggestion to rename them. I considered adding an enum class that
might confuse with Thread.State enum and so decided to leave them as
it is.
Typo: awaitArrive should be awaitAdvance
Fixed.
This updated webrev also fixes ThreadMXBeanStateTest to retry getting
ThreadInfo and improves the formatting of the output and include thread
ID in the output:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8022208/webrev.03/
This version has got 1000 successful runs of both tests with and without
-Xcomp.
thanks for the review.
Mandy
