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