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

[Yasumasa Suenaga]

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