RFR: 8265327: Remove check_safepoint_and_suspend_for_native_trans()
Daniel D.Daugherty
dcubed at openjdk.java.net
Mon Apr 19 19:45:12 UTC 2021
On Fri, 16 Apr 2021 16:24:53 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:
> Hi,
>
> Please review this small fix. Method check_safepoint_and_suspend_for_native_trans() can just be removed and the code moved directly to check_special_condition_for_native_trans(). The other caller, transition_from_native(), can also just call SafepointMechanism::process_if_requested_with_exit_check() after transitioning to _thread_in_native_trans. Since that removes the only caller of is_suspend_after_native() I also removed it.
> I also made a small fix to has_special_runtime_exit_condition(). The compilers were generating three different comparisons for _suspend_flags instead of combining the constant values and doing a single one (I checked this with gcc 10.2 on linux and clang-12 on macos). The extra code was preventing inlining on some cases. For instance, on a Linux release build with gcc 10.2, for the ThreadInVMfromNative constructor, transition_from_native() was being inlined but not process_if_requested_with_exit_check(), so we still required a call for the fast case. The small change to has_special_runtime_exit_condition() fixed that.
> Tested in mach5 tiers1-2.
>
> Thanks,
>
> Patricio
One nit and one question.
Thumbs up.
src/hotspot/share/runtime/thread.cpp line 2150:
> 2148: // pending, when _suspend_flags is non-zero or when we need to process a stack
> 2149: // watermark. Also check for pending async exceptions (except unsafe access error).
> 2150: // Note only the native==>Java barriers can call this function and when
nit typo: s/and when/when/
src/hotspot/share/runtime/thread.hpp line 1325:
> 1323: // under SR_lock in java_suspend_self().
> 1324: return (_special_runtime_exit_condition != _no_async_condition) ||
> 1325: (_suspend_flags & (_external_suspend | _obj_deopt JFR_ONLY(| _trace_flag))) != 0;
I don't see this change mentioned in the bug report and it's
not obvious why you're changing this code. Also, I think this
is going to conflict with @robehn fix for JDK-8257831.
-------------
Marked as reviewed by dcubed (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3548
More information about the hotspot-runtime-dev
mailing list