[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(S): 8086134 Deadlock detection fails to attach to core file
From:       Dmitry Samersoff <dmitry.samersoff () oracle ! com>
Date:       2015-08-25 15:11:49
Message-ID: 55DC85B5.9000103 () oracle ! com
[Download RAW message or body]

Jaroslav,
> * instead of
> `Arrays.toString(processBuilder.command().toArray()).replace(",", "")`
> you can use
> `processBuilder.command().stream().collect(Collectors.joining(" "))` -
> which is slightly more concise
> * typo 'depends *to* operating system' -> 'depends *on* operating system'

fixed. (in-place, press shift-reload).

-Dmitry



On 2015-08-21 14:44, Jaroslav Bachorik wrote:
> On 21.8.2015 12:05, Dmitry Samersoff wrote:
>> On 2015-08-21 12:35, Jaroslav Bachorik wrote:
>>> Hi Dmitry,
>>>
>>> On 19.8.2015 19:10, Dmitry Samersoff wrote:
>>>> Everybody,
>>>>
>>>> Please, review test-only fix.
>>>>
>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8086134/webrev.01/
>>>>
>>>> The test is rewritten to use testlibrary and to detect common bad
>>>> environment conditions.
>>>>
>>>> Tests in Hotspot and JDK workspaces are identical, except imports and
>>>> the fact that JDK version runs
>>>> jstack but hotspot version runs jhsdb (sa launcher).
>>>
>>> I will need more explanation here.
>>>
>>> * the issue is about the tmtools test failing; are you planning to
>>> remove the failing tests?
>>
>> Yes. Falling test have to be removed.
>>
>>> * what is the reason for having the test in hotspot and in jdk?
>>> wouldn't it be possible to have it just in jdk and launch it once
>>> using jstack and once jhsdb?
>>
>> Currently, tests for tmtools (jstack, jmap etc) resides in JDK, but
>> tests for SA resides in Hotspot.  I would love to place all tests to the
>> single place
>> but this work is out of scope of this fix.
>
> Ok. Thanks.
>
> Overall looks good. Just some minor nits.
>
> * instead of
> `Arrays.toString(processBuilder.command().toArray()).replace(",", "")`
> you can use
> `processBuilder.command().stream().collect(Collectors.joining(" "))` -
> which is slightly more concise
> * typo 'depends *to* operating system' -> 'depends *on* operating system'
>
> -JB-
>
>
>>
>> -Dmitry
>>
>>>
>>>
>>> Thanks,
>>>
>>> -JB-
>>>
>>>>
>>>> -Dmitry
>>>>
>>>>
>>>
>>
>>
>


-- 
Dmitry Samersoff 
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources. 

[prev in list] [next in list] [prev in thread] [next in thread]