RFR: 8230830: No required ResourceMark in src/hotspot/share/prims/jvmtiImpl.cpp:JvmtiSuspendControl::print()

[Leonid Mesnik]

David, Daniel, Serguei, Chris

Thank you for review.

Leonid


> On Sep 11, 2019, at 4:38 PM, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:
> 
> Thanks for chasing that down. I'm good with your change.
> 
> Dan
> 
> 
> On 9/11/19 6:45 PM, Leonid Mesnik wrote:
>> Hi
>> 
>> It is still needed for 
>> vframe *vf = vframeFor(java_thread, depth);
>> 
>> Leonid
>> 
>>> On Sep 11, 2019, at 5:56 AM, Daniel D. Daugherty <daniel.daugherty at oracle.com <mailto:daniel.daugherty at oracle.com>> wrote:
>>> 
>>> On 9/11/19 1:06 AM, Leonid Mesnik wrote:
>>>> Hi
>>>> 
>>>> Thank you for feedback.
>>>> 
>>>>> On Sep 10, 2019, at 10:03 PM, David Holmes <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>>>>> 
>>>>> Hi Leonid,
>>>>> 
>>>>> On 11/09/2019 12:03 pm, Leonid Mesnik wrote:
>>>>>> Hi
>>>>>> Could you please review following tiny fix which just add ResourceMark in JvmtiSuspendControl::print() method.
>>>>> 
>>>>> Looks fine.
>>>>> 
>>>>>> The method jvmtiSuspendControl::print() might used in custom builds only for debugging purposes. So I don't know when it was used last time. I found that it crashes when I tried to use it locally.
>>>>> 
>>>>> The only caller is JvmtiEnv::NotifyFramePop, under TraceJVMTICalls, and it already has a ResourceMark. So existing use is fine.
>>>> 
>>>> It explains why it works.
>>>  
>>> So the question that comes to my mind is whether the ResourceMark
>>> that is in JvmtiEnv::NotifyFramePop() is needed for something
>>> other than the JvmtiSuspendControl::print() call? If not, then
>>> removing the one in JvmtiEnv::NotifyFramePop() in favor of the
>>> one you added in JvmtiSuspendControl::print() is a good idea.
>>> 
>>> Dan
>>> 
>>> 
>>>> I used it to track status in suspend resume investigating https://bugs.openjdk.java.net/browse/JDK-8230459 <https://bugs.openjdk.java.net/browse/JDK-8230459>.
>>>> 
>>>>> 
>>>>> Please ensure you test with TraceJVMTICalls enabled.
>>>> 
>>>> Thanks, I have tested it with this macro enabled.
>>>> 
>>>> Leonid
>>>>> Thanks,
>>>>> David
>>>>> 
>>>>> 
>>>>>> webrev: http://cr.openjdk.java.net/~lmesnik/8230830/webrev.00/ <http://cr.openjdk.java.net/~lmesnik/8230830/webrev.00/>
>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8230830 <https://bugs.openjdk.java.net/browse/JDK-8230830>
>>>>>> Leonid
>>>> 
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190911/a4dc6b1a/attachment-0001.html>