Thread Local Handshake in JVMTI functions

[David Holmes]

On 9/04/2020 5:39 pm, Yasumasa Suenaga wrote:
> On 2020/04/09 16:19, Robbin Ehn wrote:
>> Hi Yasumasa,
>>
>> We have had internal requests doing GetThreadListStackTraces with
>> multiple threads with handshakes. Since you can sample hundreds of times
>> per second using handshakes with little interference with your
>> application.
>> The internal request sampled all threads ~10 times per second.
>> So they very much would like the performance instead of the 'precision'.

Well they won't get it with this particular API :)

If they don't want precision then they can workaround by iterating the 
threads themselves and requesting one stacktrace at a time. It won't be 
quite as performant of course, but probably better than current 
safepoint approach.

> Hmm, it seems to violate the spec of GetThreadListStackTraces():
> 
> https://docs.oracle.com/en/java/javase/13/docs/specs/jvmti.html#GetThreadListStackTraces 
> 
> ```
> All stacks are collected simultaneously, that is, no changes will occur 
> to the thread state or stacks between the sampling one thread and the next.
> ```
> 
> So I think this ticket (JDK-8242428) should just support Thread-Local 
> Handshakes when thread_count == 1.

Yes.

> For other case, we should introduce other API because current spec of 
> GetThreadListStackTraces() is useful for some case IMHO.

The current spec is critical for debugging, and can't be changed 
regardless. A new API would be needed to get non-simultaneous stacks.

Cheers,
David
-----

> For example, the profiler detects some threads which have much CPU time, 
> we can gather stack traces of them. We don't need to choose threads from 
> all thread dump.
> Of course, Both GetThreadListStackTraces() and GetAllStackTrace() use 
> safepoint, but GetThreadListStackTraces() is useful interface for 
> profiler developers. Thus I think the spec of GetThreadListStackTraces() 
> should not be changed.
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
>> Is if it's deemed feasible to always use handshakes we should consider 
>> it.
>>
>> Thanks, Robbin
>>
>> On 2020-04-09 09:12, Yasumasa Suenaga wrote:
>>> Hi Robbin,
>>>
>>> I think we can change 
>>> GetThreadListStackTrace(VM_GetThreadListStackTraces) if the caller 
>>> requests only 1 thread stack (thread_count == 1). It does not break 
>>> JVMTI spec.
>>> In other case, we should use safepoint (VM Operation) for following 
>>> JVMTI spec:
>>>
>>> ```
>>> All stacks are collected simultaneously, that is, no changes will 
>>> occur to the thread state or stacks between the sampling one thread 
>>> and the next. The threads need not be suspended.
>>> ```
>>>
>>> Thus I think we don't need describe release notes about it.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2020/04/09 16:03, Robbin Ehn wrote:
>>>> Hi, adding the same comment as in the bug regarding 
>>>> GetThreadListStackTraces.
>>>>
>>>> Please note that there is a semantic difference taking samples in a
>>>> safepoint and in handshakes, if there are mutiple thread sampled.
>>>> With a safepoint; stacktraces are taken from the same exact moment 
>>>> (from
>>>> a Java mutation perspective).
>>>>
>>>> But with handshake we can have mutation between the samples, consider:
>>>> Thread A enters a mutual exclusive region, and get sampled inside that
>>>> region.
>>>> Thread A leaves the mutual exclusive region.
>>>> Thread B enter the same mutual exclusive region, and get sampled 
>>>> inside that region.
>>>>
>>>> When looking at the stack-traces, we have have two thread inside the 
>>>> same mutual exclusive region.
>>>> If you are not aware on how the sampling mechanism works, this could 
>>>> be confusing.
>>>>
>>>> Please verify that we are still following JVM TI specs and at least 
>>>> have
>>>> information about this change in release notes.
>>>>
>>>> Thanks, Robbin
>>>>
>>>> On 2020-03-31 16:41, Daniel D. Daugherty wrote:
>>>>> Add Robbin to this thread...
>>>>>
>>>>>
>>>>> This reminded of the following RFE that Robbin filed:
>>>>>
>>>>>      JDK-8201641 JVMTI: GetThreadListStackTraces should use 
>>>>> Thread-Local Handshakes
>>>>>      https://bugs.openjdk.java.net/browse/JDK-8201641
>>>>>
>>>>> We could update 8201641 to include everything that Yasumasa-san is 
>>>>> requesting.
>>>>> Would be a good place to track it...
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>> On 3/31/20 7:40 AM, Yasumasa Suenaga wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> On 2020/03/31 19:16, David Holmes wrote:
>>>>>>> Hi Yasumasa,
>>>>>>>
>>>>>>> On 31/03/2020 8:06 pm, Yasumasa Suenaga wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> Many JVMTI functions uses VM Operation to get information. 
>>>>>>>> However some of them need to stop only one thread - they don't 
>>>>>>>> need to stop all threads.
>>>>>>>> So I think we can use Thread Local Handshake as this webrev. It 
>>>>>>>> is example for GetOneCurrentContendedMonitor().
>>>>>>>
>>>>>>> True, but at the moment handshakes involve the VMThread. There is 
>>>>>>> work being done to support direct thread-to-thread handshakes and 
>>>>>>> once that is done this kind of conversion should be more easily 
>>>>>>> done. It might be worth waiting for that.
>>>>>>
>>>>>> Thanks, I will be back to this topic when thread-to-thread 
>>>>>> handshake is done.
>>>>>> I wondered at first why VMThread involves handshake. Its 
>>>>>> improvement is welcome for me ;)
>>>>>>
>>>>>>
>>>>>> Cheers,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/jvmti-thread-local-handshake/
>>>>>>>
>>>>>>> An observation, it seems to me that calling_thread is not used 
>>>>>>> when this is not a VMOperation.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> David
>>>>>>>
>>>>>>>> Also I think we can replace following VM Operations to Thread 
>>>>>>>> Local Handshake:
>>>>>>>>
>>>>>>>> class VM_GetCurrentLocation
>>>>>>>> class VM_EnterInterpOnlyMode
>>>>>>>> class VM_UpdateForPopTopFrame
>>>>>>>> class VM_SetFramePop
>>>>>>>> class VM_GetOwnedMonitorInfo
>>>>>>>> class VM_GetCurrentContendedMonitor
>>>>>>>> class VM_GetFrameCount
>>>>>>>> class VM_GetFrameLocation
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>> It it is acceptable, I will file it to JBS and send review request.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>