PING: RFR: 8219566: JFR did not collect call stacks when MaxJavaStackTraceDepth is set to zero

[Yasumasa Suenaga]

Hi Erik,

MaxJavaStackTraceDepth do not only affect finding top stack but also stack walking.

JfrGetCallTrace::find_top_frame() uses MaxJavaStackTraceDepth to find top frame as you say.
OTOH vframeStreamSamples::samples_next() uses it to find next valid frame.


Anyway thread stack should be collected even if MaxJavaStackTraceDepth is set to zero.


Thanks,

Yasumasa


On 2019/03/23 4:25, Erik Gahlin wrote:
> To me, it seems that the purpose of the method is to find the first
> valid frame on the stack (the top frame). This is orthogonal to the
> number of frames that should be recorded by JFR.
> 
> Erik
> 
>> PING: Could you review it?
>>
>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8219566
>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8219566/webrev.02/
>>
>> This change has passed :jdk_jfr tests and submit repo.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2019/03/11 14:49, Yasumasa Suenaga wrote:
>>> 2019?3?9?(?) 20:47 Yasumasa Suenaga <yasuenag at gmail.com>:
>>>>
>>>> Hi Erik,
>>>>
>>>> Reading your email, I think we should use JfrOptionSet::stackdepth()
>>>> rather than MaxJavaStackTraceDepth.
>>>> I created new webrev, and it passed jdk_jfr jtreg tests on my Linux
>>>> x64 box.
>>>>
>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8219566/webrev.01/
>>>>
>>>> However it has not yet passed tests on submit repo
>>>> (mach5-one-ysuenaga-JDK-8219566-20190309-0909-1079793).
>>>> It seems to be failed at build phase which includes Linux.
>>>> (I can build with this change on Fedora 29 x64).
>>>>
>>>> Could you help if this webrev is ok?
>>>
>>> I found 1 warning, and I fixed it.
>>> This webrev works fine with :jdk_jfr jtreg tests on my Linux box, and
>>> submit repo test
>>> (mach5-one-ysuenaga-JDK-8219566-20190311-0150-1121570).
>>> Could you review it?
>>>
>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8219566/webrev.02/
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>
>>>> On 2019/03/09 0:37, Erik Gahlin wrote:
>>>>> JFR imposes a limit on how many frames that can be recorded, set to
>>>>> 64 by default but configurable up to 2048 using
>>>>> -XX:FlightRecorderOptions, so setting the value to 0 will still not
>>>>> mean all frames.
>>>>>
>>>>> That said, if you still want to fix it. It may make sense to set
>>>>> loop_max to INT_MAX, if MaxJavaStackTraceDepth is zero, i.e
>>>>>
>>>>> int loop_max = MaxJavaStackTraceDepth == 0 ? INT_MAX :
>>>>> MaxJavaStackTraceDepth * 2;
>>>>>
>>>>> and avoid evaluating it for every frame. Even if the compiler is
>>>>> able to hoist the comparison, it makes the code easier to read.
>>>>>
>>>>> Thanks
>>>>> Erik
>>>>>
>>>>>
>>>>>> PING: Could you review it?
>>>>>>
>>>>>>>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8219566
>>>>>>>>>     webrev:
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8219566/webrev.00/
>>>>>>
>>>>>>
>>>>>> If you have any questions, please tell me.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2019/02/27 22:46, Yasumasa Suenaga wrote:
>>>>>>> Hi Erik,
>>>>>>>
>>>>>>> On 2019/02/27 22:30, Erik Gahlin wrote:
>>>>>>>> Hi Yasumasa,
>>>>>>>>
>>>>>>>> Could you explain a little about the bug?
>>>>>>>>
>>>>>>>> Why should stack traces be collected if MaxJavaStackTraceDepth
>>>>>>>> is set to 0.
>>>>>>>
>>>>>>> "0" in MaxJavaStackTraceDepth means all exception stack traces
>>>>>>> will be shown.
>>>>>>>
>>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/72ce7dd54939/src/hotspot/share/runtime/globals.hpp#l1553
>>>>>>>
>>>>>>>
>>>>>>> JDK-7179701 reports similar issue.
>>>>>>> I think we can take similar approach for JDK-8219566.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Erik
>>>>>>>>
>>>>>>>>> On 22 Feb 2019, at 07:48, Yasumasa Suenaga <yasuenag at gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> Please review this change:
>>>>>>>>>
>>>>>>>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8219566
>>>>>>>>>     webrev:
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8219566/webrev.00/
>>>>>>>>>
>>>>>>>>> JFR did not collect call stacks when MaxJavaStackTraceDepth is
>>>>>>>>> set to zero.
>>>>>>>>> This bug is similar to JDK-7179701.
>>>>>>>>>
>>>>>>>>> This change passed tests on submit repo and :jdk_jfr jtreg tests.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>
>>>>>
>