Implement suspend flags as atomic variable (#2361)

We have observed a significant performance degradation after merging
#1991
Instead of protecting suspend flags with a mutex, we implement the flags
as atomic variable and only use mutex when atomics are not available
on a given platform.

Author: loganek

core/iwasm/common/wasm_exec_env.h

@@ -7,6 +7,7 @@
 #define _WASM_EXEC_ENV_H
 
 #include "bh_assert.h"
+#include "wasm_suspend_flags.h"
 #if WASM_ENABLE_INTERP != 0
 #include "../interpreter/wasm.h"
 #endif
@@ -57,15 +58,8 @@ typedef struct WASMExecEnv {
        exception. */
     uint8 *native_stack_boundary;
 
-    /* Used to terminate or suspend current thread
-        bit 0: need to terminate
-        bit 1: need to suspend
-        bit 2: need to go into breakpoint
-        bit 3: return from pthread_exit */
-    union {
-        uint32 flags;
-        uintptr_t __padding__;
-    } suspend_flags;
+    /* Used to terminate or suspend current thread */
+    WASMSuspendFlags suspend_flags;
 
     /* Auxiliary stack boundary */
     union {

core/iwasm/common/wasm_suspend_flags.h

@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2023 Amazon Inc.  All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ */
+
+#ifndef _WASM_SUSPEND_FLAGS_H
+#define _WASM_SUSPEND_FLAGS_H
+
+#include "bh_platform.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Need to terminate */
+#define WASM_SUSPEND_FLAG_TERMINATE 0x1
+/* Need to suspend */
+#define WASM_SUSPEND_FLAG_SUSPEND 0x2
+/* Need to go into breakpoint */
+#define WASM_SUSPEND_FLAG_BREAKPOINT 0x4
+/* Return from pthread_exit */
+#define WASM_SUSPEND_FLAG_EXIT 0x8
+
+typedef union WASMSuspendFlags {
+    uint32 flags;
+    uintptr_t __padding__;
+} WASMSuspendFlags;
+
+#if defined(__GNUC_PREREQ)
+#if __GNUC_PREREQ(4, 7)
+#define CLANG_GCC_HAS_ATOMIC_BUILTIN
+#endif
+#elif defined(__clang__)
+#if __clang_major__ > 3 || (__clang_major__ == 3 && __clang_minor__ >= 0)
+#define CLANG_GCC_HAS_ATOMIC_BUILTIN
+#endif
+#endif
+
+#if defined(CLANG_GCC_HAS_ATOMIC_BUILTIN)
+#define WASM_SUSPEND_FLAGS_IS_ATOMIC 1
+#define WASM_SUSPEND_FLAGS_GET(s_flags) \
+    __atomic_load_n(&s_flags.flags, __ATOMIC_SEQ_CST)
+#define WASM_SUSPEND_FLAGS_FETCH_OR(s_flags, val) \
+    __atomic_fetch_or(&s_flags.flags, val, __ATOMIC_SEQ_CST)
+#define WASM_SUSPEND_FLAGS_FETCH_AND(s_flags, val) \
+    __atomic_fetch_and(&s_flags.flags, val, __ATOMIC_SEQ_CST)
+#else /* else of defined(CLANG_GCC_HAS_ATOMIC_BUILTIN) */
+#define WASM_SUSPEND_FLAGS_GET(s_flags) (s_flags.flags)
+#define WASM_SUSPEND_FLAGS_FETCH_OR(s_flags, val) (s_flags.flags |= val)
+#define WASM_SUSPEND_FLAGS_FETCH_AND(s_flags, val) (s_flags.flags &= val)
+
+/* The flag can be defined by the user if the platform
+   supports atomic access to uint32 aligned memory. */
+#ifdef WASM_UINT32_IS_ATOMIC
+#define WASM_SUSPEND_FLAGS_IS_ATOMIC 1
+#else /* else of WASM_UINT32_IS_ATOMIC */
+#define WASM_SUSPEND_FLAGS_IS_ATOMIC 0
+#endif /* WASM_UINT32_IS_ATOMIC */
+
+#endif
+
+#if WASM_SUSPEND_FLAGS_IS_ATOMIC != 0
+#define WASM_SUSPEND_FLAGS_LOCK(lock) (void)0
+#define WASM_SUSPEND_FLAGS_UNLOCK(lock) (void)0
+#else /* else of WASM_SUSPEND_FLAGS_IS_ATOMIC */
+#define WASM_SUSPEND_FLAGS_LOCK(lock) os_mutex_lock(&lock)
+#define WASM_SUSPEND_FLAGS_UNLOCK(lock) os_mutex_unlock(&lock);
+#endif /* WASM_SUSPEND_FLAGS_IS_ATOMIC */
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* end of _WASM_SUSPEND_FLAGS_H */

core/iwasm/interpreter/wasm_interp_classic.c

@@ -1062,21 +1062,33 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
         os_mutex_unlock(&exec_env->wait_lock);                         \
     } while (0)
 #else
-#define CHECK_SUSPEND_FLAGS()                                             \
-    do {                                                                  \
-        os_mutex_lock(&exec_env->wait_lock);                              \
-        if (exec_env->suspend_flags.flags != 0) {                         \
-            if (exec_env->suspend_flags.flags & 0x01) {                   \
-                /* terminate current thread */                            \
-                os_mutex_unlock(&exec_env->wait_lock);                    \
-                return;                                                   \
-            }                                                             \
-            while (exec_env->suspend_flags.flags & 0x02) {                \
-                /* suspend current thread */                              \
-                os_cond_wait(&exec_env->wait_cond, &exec_env->wait_lock); \
-            }                                                             \
-        }                                                                 \
-        os_mutex_unlock(&exec_env->wait_lock);                            \
+#if WASM_SUSPEND_FLAGS_IS_ATOMIC != 0
+/* The lock is only needed when the suspend_flags is atomic; otherwise
+   the lock is already taken at the time when SUSPENSION_LOCK() is called. */
+#define SUSPENSION_LOCK() os_mutex_lock(&exec_env->wait_lock);
+#define SUSPENSION_UNLOCK() os_mutex_unlock(&exec_env->wait_lock);
+#else
+#define SUSPENSION_LOCK()
+#define SUSPENSION_UNLOCK()
+#endif
+
+#define CHECK_SUSPEND_FLAGS()                                         \
+    do {                                                              \
+        WASM_SUSPEND_FLAGS_LOCK(exec_env->wait_lock);                 \
+        if (WASM_SUSPEND_FLAGS_GET(exec_env->suspend_flags)           \
+            & WASM_SUSPEND_FLAG_TERMINATE) {                          \
+            /* terminate current thread */                            \
+            WASM_SUSPEND_FLAGS_UNLOCK(exec_env->wait_lock);           \
+            return;                                                   \
+        }                                                             \
+        while (WASM_SUSPEND_FLAGS_GET(exec_env->suspend_flags)        \
+               & WASM_SUSPEND_FLAG_SUSPEND) {                         \
+            /* suspend current thread */                              \
+            SUSPENSION_LOCK()                                         \
+            os_cond_wait(&exec_env->wait_cond, &exec_env->wait_lock); \
+            SUSPENSION_UNLOCK()                                       \
+        }                                                             \
+        WASM_SUSPEND_FLAGS_UNLOCK(exec_env->wait_lock);               \
     } while (0)
 #endif /* WASM_ENABLE_DEBUG_INTERP */
 #endif /* WASM_ENABLE_THREAD_MGR */
@@ -3783,7 +3795,8 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
             HANDLE_OP(DEBUG_OP_BREAK)
             {
                 wasm_cluster_thread_send_signal(exec_env, WAMR_SIG_TRAP);
-                exec_env->suspend_flags.flags |= 2;
+                WASM_SUSPEND_FLAGS_FETCH_OR(exec_env->suspend_flags,
+                                            WASM_SUSPEND_FLAG_SUSPEND);
                 frame_ip--;
                 SYNC_ALL_TO_FRAME();
                 CHECK_SUSPEND_FLAGS();

core/iwasm/interpreter/wasm_interp_fast.c

@@ -1065,18 +1065,17 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
 #endif
 
 #if WASM_ENABLE_THREAD_MGR != 0
-#define CHECK_SUSPEND_FLAGS()                           \
-    do {                                                \
-        os_mutex_lock(&exec_env->wait_lock);            \
-        if (exec_env->suspend_flags.flags != 0) {       \
-            if (exec_env->suspend_flags.flags & 0x01) { \
-                /* terminate current thread */          \
-                os_mutex_unlock(&exec_env->wait_lock);  \
-                return;                                 \
-            }                                           \
-            /* TODO: support suspend and breakpoint */  \
-        }                                               \
-        os_mutex_unlock(&exec_env->wait_lock);          \
+#define CHECK_SUSPEND_FLAGS()                               \
+    do {                                                    \
+        WASM_SUSPEND_FLAGS_LOCK(exec_env->wait_lock);       \
+        if (WASM_SUSPEND_FLAGS_GET(exec_env->suspend_flags) \
+            & WASM_SUSPEND_FLAG_TERMINATE) {                \
+            /* terminate current thread */                  \
+            WASM_SUSPEND_FLAGS_UNLOCK(exec_env->wait_lock); \
+            return;                                         \
+        }                                                   \
+        /* TODO: support suspend and breakpoint */          \
+        WASM_SUSPEND_FLAGS_UNLOCK(exec_env->wait_lock);     \
     } while (0)
 #endif
 

core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c

@@ -531,7 +531,8 @@ pthread_start_routine(void *arg)
     else {
         info_node->u.ret = (void *)(uintptr_t)argv[0];
 #ifdef OS_ENABLE_HW_BOUND_CHECK
-        if (exec_env->suspend_flags.flags & 0x08)
+        if (WASM_SUSPEND_FLAGS_GET(exec_env->suspend_flags)
+            & WASM_SUSPEND_FLAG_EXIT)
             /* argv[0] isn't set after longjmp(1) to
                invoke_native_with_hw_bound_check */
             info_node->u.ret = exec_env->thread_ret_value;

core/iwasm/libraries/thread-mgr/thread_manager.c

@@ -606,7 +606,8 @@ thread_manager_start_routine(void *arg)
 
 #ifdef OS_ENABLE_HW_BOUND_CHECK
     os_mutex_lock(&exec_env->wait_lock);
-    if (exec_env->suspend_flags.flags & 0x08)
+    if (WASM_SUSPEND_FLAGS_GET(exec_env->suspend_flags)
+        & WASM_SUSPEND_FLAG_EXIT)
         ret = exec_env->thread_ret_value;
     os_mutex_unlock(&exec_env->wait_lock);
 #endif
@@ -993,7 +994,9 @@ wasm_cluster_exit_thread(WASMExecEnv *exec_env, void *retval)
     if (exec_env->jmpbuf_stack_top) {
         /* Store the return value in exec_env */
         exec_env->thread_ret_value = retval;
-        exec_env->suspend_flags.flags |= 0x08;
+
+        WASM_SUSPEND_FLAGS_FETCH_OR(exec_env->suspend_flags,
+                                    WASM_SUSPEND_FLAG_EXIT);
 
 #ifndef BH_PLATFORM_WINDOWS
         /* Pop all jmpbuf_node except the last one */
@@ -1055,7 +1058,8 @@ set_thread_cancel_flags(WASMExecEnv *exec_env)
 #if WASM_ENABLE_DEBUG_INTERP != 0
     wasm_cluster_thread_send_signal(exec_env, WAMR_SIG_TERM);
 #endif
-    exec_env->suspend_flags.flags |= 0x01;
+    WASM_SUSPEND_FLAGS_FETCH_OR(exec_env->suspend_flags,
+                                WASM_SUSPEND_FLAG_TERMINATE);
 
     os_mutex_unlock(&exec_env->wait_lock);
 }
@@ -1178,7 +1182,8 @@ void
 wasm_cluster_suspend_thread(WASMExecEnv *exec_env)
 {
     /* Set the suspend flag */
-    exec_env->suspend_flags.flags |= 0x02;
+    WASM_SUSPEND_FLAGS_FETCH_OR(exec_env->suspend_flags,
+                                WASM_SUSPEND_FLAG_SUSPEND);
 }
 
 static void
@@ -1214,7 +1219,8 @@ wasm_cluster_suspend_all_except_self(WASMCluster *cluster,
 void
 wasm_cluster_resume_thread(WASMExecEnv *exec_env)
 {
-    exec_env->suspend_flags.flags &= ~0x02;
+    WASM_SUSPEND_FLAGS_FETCH_AND(exec_env->suspend_flags,
+                                 ~WASM_SUSPEND_FLAG_SUSPEND);
     os_cond_signal(&exec_env->wait_cond);
 }
 
@@ -1343,8 +1349,10 @@ bool
 wasm_cluster_is_thread_terminated(WASMExecEnv *exec_env)
 {
     os_mutex_lock(&exec_env->wait_lock);
-    bool is_thread_terminated =
-        (exec_env->suspend_flags.flags & 0x01) ? true : false;
+    bool is_thread_terminated = (WASM_SUSPEND_FLAGS_GET(exec_env->suspend_flags)
+                                 & WASM_SUSPEND_FLAG_TERMINATE)
+                                    ? true
+                                    : false;
     os_mutex_unlock(&exec_env->wait_lock);
 
     return is_thread_terminated;

core/shared/utils/bh_platform.h

@@ -16,6 +16,7 @@
 #include "bh_log.h"
 #include "bh_queue.h"
 #include "bh_vector.h"
+#include "gnuc.h"
 #include "runtime_timer.h"
 
 /**