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

[Erik Gahlin]

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