[ping] Re: RFR 8058506: ThreadMXBeanStateTest throws exception

[Jaroslav Bachorik]

On 11/04/2014 07:50 AM, David Holmes wrote:
> On 3/11/2014 10:37 PM, Jaroslav Bachorik wrote:
>> May I have a (R)eviewer to take a look at this, please?
>>
>> http://cr.openjdk.java.net/~jbachorik/8058506/webrev.01
>
> I don't understand the need for the semaphore given you do:
>
>   116             thread.join();
>   117             System.out.println(thread.getLog());
>
> If join() returns normally, such that you call getLog() then run() must
> have completed and the semaphore has to be signalled. So the semaphore
> is serving no purpose here that I can see. If you want to ensure
> getLog() blocks until the thread has terminated then it could internally
> do a join().

I prefer having the precondition of "getLog()" method enforced inside 
the ThreadStateController class and not to rely on the caller to do the 
right thing.

I've replaced the semaphore with the plain thrd.join() call as suggested.

http://cr.openjdk.java.net/~jbachorik/8058506/webrev.02

-JB-

>
> David
> -----
>
> David
>
>> -JB-
>>
>> On 10/23/2014 03:31 PM, Erik Gahlin wrote:
>>> Looks good!
>>>
>>> Erik
>>>
>>> Jaroslav Bachorik skrev 2014-10-23 11:45:
>>>> On 10/21/2014 07:47 PM, Jaroslav Bachorik wrote:
>>>>> On 10/21/2014 03:27 PM, Erik Gahlin wrote:
>>>>>> Have you considered creating a LogMessage class that keeps the
>>>>>> logCntr
>>>>>> value and the log message, instead of putting the counter into the
>>>>>> log
>>>>>> string and parsing it.
>>>>>
>>>>> Yes. And didn't go that way in order to prevent creating a lot of
>>>>> throwaway stringbuilder instances (the Formatter works that way) - but
>>>>> it might (almost certainly) be a premature optimization. I will
>>>>> clean it
>>>>> up and resubmit the request.
>>>>
>>>> http://cr.openjdk.java.net/~jbachorik/8058506/webrev.01
>>>>
>>>> I moved the log collection logic to a separate class available in the
>>>> testlib now. The code is much more concise now.
>>>>
>>>> -JB-
>>>>
>>>>>
>>>>> -JB-
>>>>>
>>>>>>
>>>>>> Seems simpler and easier to understand.
>>>>>>
>>>>>> Erik
>>>>>>
>>>>>> Jaroslav Bachorik skrev 2014-10-20 13:12:
>>>>>>> Please, review the following test change
>>>>>>>
>>>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8058506
>>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8058506/webrev.00
>>>>>>>
>>>>>>> The test fails intermittently due to the log printing blocking the
>>>>>>> test thread from time to time, resulting in incorrect data
>>>>>>> reported by
>>>>>>> ThreadMXBean.
>>>>>>>
>>>>>>> The solution is to use per-thread non-blocking StringBuilders
>>>>>>> (wrapped
>>>>>>> in Formatter instances) and aggregate the log output only after the
>>>>>>> test is finished.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> -JB-
>>>>>>
>>>>>
>>>>
>>>
>>