RFR (XS) 8222297: IRT_ENTRY/IRT_LEAF etc are the same as JRT

Daniel D. Daugherty daniel.daugherty at oracle.com
Fri Apr 12 13:31:00 UTC 2019


> > I'm not clear what your resolution is here? Just accept that maybe 
> we lost a verify-gc call as Dan noted?
>
> I think there is no actual lost verify call, in that there is no IRT 
> entry coming from native (that I can spot visually without running 
> verification),

I could not spot any IRT_LEAF coming from native either.

I'm good with pushing this change and keeping an eye open for any 
anomalies...

Thumbs up!

Dan


On 4/12/19 7:39 AM, coleen.phillimore at oracle.com wrote:
>
>
> On 4/11/19 7:24 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> I was busy doing some archaeology on this code so didn't notice the 
>> RFR. Glad Dan picked up on the only difference with the "verifiers" 
>> in the LEAF variants.
>>
>> FTR the differences here are historical. JRT was added first and 
>> shortly needed to manifest the current thread directly. IRT were 
>> added later and the "thread" was an implicit argument. But by July 
>> 1998 the two ENTRY macros were the same. The only difference was the 
>> verifier in the LEAF, and some custom variants of each macro that no 
>> longer exist.
>>
>> Conceptually I've always thought there was a difference in how the 
>> interpreter needed to "enter the runtime" versus the compilers. So 
>> the different macros made that clear. But if the requirements are 
>> essentially identical, and always have been, then the distinction 
>> just confuses things.
>
> Right.  And as I put it in the bug, there have been some extensions to 
> the JRT entries that every now and then, I think I need for the 
> interpreter, so these distinctions are just confusing.
>>
>> I'm not clear what your resolution is here? Just accept that maybe we 
>> lost a verify-gc call as Dan noted?
>
> I think there is no actual lost verify call, in that there is no IRT 
> entry coming from native (that I can spot visually without running 
> verification), and I can't figure why out it's makes sense to verify 
> that there's no GC, when we've already verified that there is no 
> safepoint.
>
> NoGCVerifier::NoGCVerifier(bool verifygc) {
>   _verifygc = verifygc;
>   if (_verifygc) {
>     CollectedHeap* h = Universe::heap();
>     assert(!h->is_gc_active(), "GC active during NoGCVerifier");
>     _old_invocations = h->total_collections();
>   }
>
> When is_gc_active is set here:
>
> void GenCollectedHeap::do_collection(bool           full,
> ...
>   assert(SafepointSynchronize::is_at_safepoint(), "should be at 
> safepoint");
> ...
>   FlagSetting fl(_is_gc_active, true);
>
> Except for ZGC, which I can't tell, increment_total_collections also 
> is called at a safepoint.
>
> It might be a useful assert if we want to prevent checking for the 
> start of a concurrent collection.  If the thread is in native, it 
> doesn't actually make sense because in native should not have any 
> direct access to oops or metadata.  So if the verification is actually 
> lost, which I doubt, that's a good thing.
>
> Coleen
>
>>
>> Thanks,
>> David
>>
>> On 12/04/2019 7:06 am, coleen.phillimore at oracle.com wrote:
>>>
>>> Dan, Thank you for reviewing.
>>>
>>> On 4/11/19 3:02 PM, Daniel D. Daugherty wrote:
>>>> On 4/11/19 11:39 AM, coleen.phillimore at oracle.com wrote:
>>>>> Summary: Replace IRT entry points with JRT.
>>>>>
>>>>> Tested with hs tier1-3 and built zero.  And grepped from the right 
>>>>> level directory this time.
>>>>>
>>>>> open webrev at 
>>>>> http://cr.openjdk.java.net/~coleenp/2019/8222297.01/webrev
>>>>
>>>> src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/arm/interpreterRT_arm.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/ppc/interpreterRT_ppc.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/s390/interpreterRT_s390.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/sparc/interpreterRT_sparc.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/x86/interpreterRT_x86_32.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/x86/interpreterRT_x86_64.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/zero/cppInterpreter_zero.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/zero/interpreterRT_zero.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/share/interpreter/interpreterRuntime.cpp
>>>> src/hotspot/share/runtime/interfaceSupport.inline.hpp
>>>>     old L435: #define IRT_LEAF(result_type, header)
>>>>     old L438:     debug_only(NoSafepointVerifier __nspv(true);)
>>>>     new L432: #define JRT_LEAF(result_type, header)
>>>>     new L435:   debug_only(JRTLeafVerifier __jlv;)
>>>>         src/hotspot/share/runtime/interfaceSupport.cpp:
>>>>
>>>>           JRTLeafVerifier::JRTLeafVerifier()
>>>>             : NoSafepointVerifier(true, 
>>>> JRTLeafVerifier::should_verify_GC())
>>>>           {
>>>>           }
>>>>
>>>>         src/hotspot/share/runtime/safepointVerifiers.hpp:
>>>>
>>>>           NoSafepointVerifier(bool activated = true, bool verifygc 
>>>> = true ) :
>>>>             NoGCVerifier(verifygc),
>>>>             _activated(activated) {
>>>>
>>>>         IRT_LEAF creates a NoSafepointVerifier with first ctr param 
>>>> == true
>>>>         and the second ctr param == default true.
>>>>
>>>>         JRT_LEAF creates a JRTLeafVerifier subclassed on 
>>>> NoSafepointVerifier
>>>>         with first ctr param == true and second ctr param based on
>>>>         JRTLeafVerifier::should_verify_GC() which can return either
>>>>         true or false depending on the calling thread's state. If the
>>>>         thread's state == _thread_in_Java, then the return == true.
>>>>         If the thread's state == _thread_in_native, then the return 
>>>> == false.
>>>>
>>>>         As long as all the IRT_LEAF uses are thread state == 
>>>> _thread_in_Java
>>>>         then this is an equivalent change.
>>>>
>>>>         I found these uses of IRT_LEAF:
>>>>
>>>>           SharedRuntime::fixup_callers_callsite()
>>>
>>> This is called from the c2i adapter so would be thread_in_java.
>>>
>>>>     InterpreterRuntime::bcp_to_di()
>>>>           InterpreterRuntime::verify_mdp()
>>>>           InterpreterRuntime::interpreter_contains()
>>>>           InterpreterRuntime::popframe_move_outgoing_args()
>>>>           InterpreterRuntime::trace_bytecode()
>>>>
>>>
>>> I assume these are thread_in_java too, since they have access to 
>>> metadata (except interpreter_contains, whose callers have access to 
>>> metadata).  This would be dangerous for in_native threads to touch 
>>> metadata directly.
>>>
>>>> I have not checked to see if these IRT_LEAF functions are
>>>>         ever called from thread state == _thread_in_native locations,
>>>>         but if they are, then we will no longer 'verifygc' with the
>>>>         JRT_LEAF switch.
>>>>
>>> It seems like if one of these IRT calls was from native the
>>>
>>> NoSafepointVerifier( true /* no safepoints */, false /* don't verify 
>>> GC */) might be a fix to these methods.  Because of the comment:
>>>
>>>    case _thread_in_native:
>>>      // A native thread is not subject to safepoints.
>>>      // Even while it is in a leaf routine, GC is ok
>>>      return false;
>>>
>>> But I think it's the case that the behavior is the same, or more 
>>> consistent if one of these is a native LEAF call that had IRT_LEAF. 
>>> Trying to resolve these subtle differences is the trouble with 
>>> mostly duplicated code :(
>>>
>>> Thanks!
>>> Coleen
>>>
>>>> src/hotspot/share/runtime/sharedRuntime.cpp
>>>>     No comments.
>>>>
>>>>
>>>> Your call on what to do about the difference that I found between
>>>> IRT_LEAF and JRT_LEAF. We could be losing a 'verifygc' check here,
>>>> but...
>>>>
>>>> Dan
>>>>
>>>>
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8222297
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>
>>>
>



More information about the hotspot-runtime-dev mailing list