RFR 8159284: bigapps/Jetty - assert(jfa->last_Java_sp() > sp()) failed with tracing code in use

[Frederic Parain]

Looks good to me.
Thank you for the renaming.

Fred

On 08/02/2016 06:19 PM, Coleen Phillimore wrote:
>
>
> On 8/2/16 6:08 PM, Coleen Phillimore wrote:
>>
>> Hi, I have another webrev with the name change of
>> is_entry_frame_valid(), which passes JPRT.
>
> http://cr.openjdk.java.net/~coleenp/8159284.02/webrev/index.html
>
>>
>> thanks,
>> Coleen
>>
>>
>> On 8/2/16 2:28 PM, Frederic Parain wrote:
>>>
>>>
>>> On 08/02/2016 02:22 PM, Coleen Phillimore wrote:
>>>>
>>>> Fred, Thank you for reviewing this.
>>>>
>>>> On 8/2/16 2:02 PM, Frederic Parain wrote:
>>>>> Coleen,
>>>>>
>>>>> src/cpu/aarch64/vm/frame_aarch64.cpp
>>>>>
>>>>>   line 116: return entry_frame_is_safe(thread);
>>>>>   line 207: return sender.is_entry_frame_safe(thread);
>>>>>
>>>>>   Method names are different, is it a typo?
>>>>>
>>>>
>>>> Yes, it is a typo.  Thank you for catching it.
>>>>
>>>>> src/cpu/sparc/vm/frame_sparc.cpp
>>>>>
>>>>>   No comments
>>>>>
>>>>> src/cpu/x86/vm/frame_x86.cpp
>>>>>
>>>>>   No comments
>>>>>
>>>>> src/share/vm/runtime/frame.cpp
>>>>>
>>>>>   No comments
>>>>>
>>>>> src/share/vm/runtime/frame.hpp
>>>>>
>>>>>   I would expect the new method name to be is_entry_frame_safe()
>>>>>   as most tester methods begin with "is_" but I this is just a
>>>>>   personal opinion.
>>>>
>>>> I named it entry_frame_is_safe() because I thought it looked less
>>>> redundant and more distinct in the code fragments where they occurred:
>>>>
>>>>     // Entry frame checks
>>>>     if (is_entry_frame()) {
>>>>       // an entry frame must have a valid fp.
>>>>
>>>>       if (!fp_safe) return false;
>>>>
>>>>       return entry_frame_is_safe(thread);
>>>>     }
>>>>
>>>> is_entry_frame_safe() looks like is_entry_frame() or implies it's a
>>>> "safe" call to is_entry_frame.
>>>>
>>>> I guess it is the standard to start with 'is' for these sorts of
>>>> tests.   How about is_safe_entry_frame() ?
>>>
>>> Sounds good.
>>>
>>> Fred
>>>
>>>> thanks,
>>>> Coleen
>>>>
>>>>>
>>>>> Fred
>>>>>
>>>>> On 08/01/2016 07:38 PM, Coleen Phillimore wrote:
>>>>>> Summary: Test condition in assert in frame::safe_for_sender() for
>>>>>> entry
>>>>>> frames and return false.
>>>>>>
>>>>>> This bug is for a confidential part of the project that needs more
>>>>>> robustness checks to verify that frame::sender() can be called.
>>>>>>
>>>>>> Also refactored into frame::entry_frame_is_safe() because these
>>>>>> platforms had the same code for entry frames to determine whether
>>>>>> it is
>>>>>> still safe to trust this frame to call sender(). These platforms had
>>>>>> also the same assert in sender_for_entry_frame() that the sampling
>>>>>> code
>>>>>> hit.
>>>>>>
>>>>>> Tested with our nightly tests on all platforms and bigapps/Jetty.
>>>>>>
>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8159284.01/webrev
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>>>
>>>>
>>
>