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