[ping] Re: RFR 8058506: ThreadMXBeanStateTest throws exception

[David Holmes]

On 4/11/2014 7:49 PM, Jaroslav Bachorik wrote:
> 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.

Ok. But then the test doesn't need the join() any more. :)

Seems okay though - Reviewed.

Thanks,
David

> 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-
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>