[ping] Re: RFR 8058506: ThreadMXBeanStateTest throws exception

[Jaroslav Bachorik]

On 11/04/2014 11:56 AM, David Holmes wrote:
> 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. :)

Sounds fair :)

-JB-

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