RFR (XS) 8222297: IRT_ENTRY/IRT_LEAF etc are the same as JRT
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Fri Apr 12 13:59:17 UTC 2019
Thanks Dan!
Coleen
On 4/12/19 9:31 AM, Daniel D. Daugherty wrote:
>> > 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