RFR(S): 8063112: Compiler diagnostic commands should have locking instead of safepoint

[Nils Eliasson]

Thanks Vladimir!

Regards,
Nils

On 2016-01-27 21:57, Vladimir Kozlov wrote:
> Yes, this looks much better. Reviewed.
>
> thanks,
> Vladimir
>
> On 1/26/16 2:40 AM, Nils Eliasson wrote:
>> Hi Vladimir,
>>
>> On 2016-01-22 20:23, Vladimir Kozlov wrote:
>>> Why you need new print method? Why you can't use existing print()?
>>> Also I prefer to get current compilation tasks print in separate lines
>>> - not in the list of threads. Then you don't need to use new print?
>>
>> Works for me. I moved it directly after the existing thread printing:
>>
>> ---------------  P R O C E S S  ---------------
>>
>> Java Threads: ( => current thread )
>>    0x00007f4cfc485000 JavaThread "Service Thread" daemon
>> [_thread_blocked, id=22409, 
>> stack(0x00007f4bf1c5e000,0x00007f4bf1d5f000)]
>>    0x00007f4cfc476000 JavaThread "Sweeper thread" daemon
>> [_thread_blocked, id=22408, 
>> stack(0x00007f4bf1d5f000,0x00007f4bf1e60000)]
>> ...
>> stack(0x00007f4bf35db000,0x00007f4bf36dc000)]
>>    0x00007f4cfc018800 JavaThread "main" [_thread_in_vm, id=22332,
>> stack(0x00007f4d05c78000,0x00007f4d05d79000)]
>>
>> Other Threads:
>>    0x00007f4cfc3ea000 VMThread [stack:
>> 0x00007f4bf36dc000,0x00007f4bf37dd000] [id=22388]
>>    0x00007f4cfc486800 WatcherThread [stack:
>> 0x00007f4bf1b5d000,0x00007f4bf1c5e000] [id=22410]
>>
>> Threads with active compile tasks:
>>    0x00007f4cfc46a800 id=22403 Compiling:     244    1       3
>> java.lang.String::isLatin1 (19 bytes)
>>
>>
>>>
>>> I am worry about using locks for printing because print code also has
>>> locks. Do we really have to have locks here? The output for these
>>> directives is local bufferedStream. As I understand it is separate for
>>> each directive. So why you need lock? Or VM operation as before?
>>
>> I think you are mixing my two RFRs together - this change doesn't print
>> directives.
>>
>> I am removing vm_ops from three diagnostic commands that uses code that
>> expects safepoint or lock. Some of the commands are really quick, and
>> requesting a safepoint is overkill when it can be done concurrently.
>> Only new lock taken is the thread lock when iterating the compiler
>> threads from the Compiler.queue jcmd. The thread lock is ranked so it
>> can not be reordered with the compile.queue lock.
>>
>> I cleaned it up a bit further and removed the unused
>> print_compiler_threads_on(...) from compileBroker. It is printed in
>> JavaThread::print_on(..) where all the other thread info is located.
>>
>> Hs_err-file looks like the example above.
>>
>> jcmd Thread.print looks like this for compiling threads:
>>
>> C1 CompilerThread13" #19 daemon prio=9 os_prio=0 tid=0x00007f8748471800
>> nid=0x7732 runnable [0x0000000000000000]
>>     java.lang.Thread.State: RUNNABLE
>>     JavaThread state: _thread_in_native
>> Thread: 0x00007f8748471800  [0x7732] State: _at_safepoint
>> _has_called_back 0 _at_poll_safepoint 0
>>     JavaThread state: _thread_in_native
>>     Compiling:  716    b  2 java.util.regex.Pattern::compile (406
>> bytes)
>>
>> And Compiler.queue looks like this:
>>
>> "Current compiles:
>> C1 CompilerThread14 435    b  2 java.net.URLStreamHandler::parseURL
>> (1166 bytes)
>>
>> C1 compile queue:
>> Empty
>>
>> C2 compile queue:
>> Empty"
>>
>>
>> New webrev: http://cr.openjdk.java.net/~neliasso/8063112/webrev.04/
>>
>> Regards,
>> Nils
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 1/22/16 6:40 AM, Nils Eliasson wrote:
>>>> Hi,
>>>>
>>>> Please review.
>>>>
>>>> Summary:
>>>> Firstly this change removes the unnecessary vm-ops from three compiler
>>>> diagnostic commands and adds locking instead.
>>>> Secondly the Compiler.queue diagnostic command is improved with 
>>>> printing
>>>> of any active compilations. I found this useful when diagnosing a
>>>> rouge VM.
>>>> Thirdly, as a bonus, I also add printing of active compilations in the
>>>> thread section of the hs_err file. Very useful when investigating VMs
>>>> terminated by a timeout.
>>>>
>>>> Testing:
>>>> This does not pass all tests yet. A few tests is dependent on the 
>>>> output
>>>> from the diagnostic command, and I want to be sure the reviewers are
>>>> happy with the output format first.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8063112
>>>> Webrev: http://cr.openjdk.java.net/~neliasso/8063112/webrev.02/
>>>>
>>>> Regards,
>>>> Nils
>>>>
>>