Skip to content

Commit

Permalink
debug-interp: Only add lock when signal_flag is SIG_SINGSTEP (bytecod…
Browse files Browse the repository at this point in the history 
…ealliance#3704)

As reported in bytecodealliance#3500, when debug interpreter is enabled, the classic interpreter
performs a lock operation to read `exec_env->current_status->signal_flag` and
do further handling before fetching next opcode, which makes the interpreter
run slower.

This PR atomic loads the `exec_env->current_status->signal_flag` without mutex
lock when 32-bit atomic load is supported, and only adding lock for further
handling when the signal_flag is WAMR_SIG_SINGSTEP, which improves the
performance.
  • Loading branch information
@wenyongh
wenyongh authored Aug 14, 2024
1 parent 37d7439 commit da25906
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 26 deletions.
79 changes: 56 additions & 23 deletions core/iwasm/interpreter/wasm_interp_classic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1399,17 +1399,64 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
#endif /* WASM_ENABLE_DEBUG_INTERP */
#endif /* WASM_ENABLE_THREAD_MGR */

#if WASM_ENABLE_THREAD_MGR != 0 && WASM_ENABLE_DEBUG_INTERP != 0
#if BH_ATOMIC_32_IS_ATOMIC != 0
#define GET_SIGNAL_FLAG() \
do { \
signal_flag = \
BH_ATOMIC_32_LOAD(exec_env->current_status->signal_flag); \
} while (0)
#else
#define GET_SIGNAL_FLAG() \
do { \
os_mutex_lock(&exec_env->wait_lock); \
signal_flag = exec_env->current_status->signal_flag; \
os_mutex_unlock(&exec_env->wait_lock); \
} while (0)
#endif
#endif

#if WASM_ENABLE_LABELS_AS_VALUES != 0

#define HANDLE_OP(opcode) HANDLE_##opcode:
#define FETCH_OPCODE_AND_DISPATCH() goto *handle_table[*frame_ip++]

#if WASM_ENABLE_THREAD_MGR != 0 && WASM_ENABLE_DEBUG_INTERP != 0
#define HANDLE_OP_END() \
do { \
/* Record the current frame_ip, so when exception occurs, \
debugger can know the exact opcode who caused the exception */ \
frame_ip_orig = frame_ip; \
/* Atomic load the exec_env's signal_flag first, and then handle \
more with lock if it is WAMR_SIG_SINGSTEP */ \
GET_SIGNAL_FLAG(); \
if (signal_flag == WAMR_SIG_SINGSTEP) { \
os_mutex_lock(&exec_env->wait_lock); \
while (exec_env->current_status->signal_flag == WAMR_SIG_SINGSTEP \
&& exec_env->current_status->step_count++ == 1) { \
exec_env->current_status->step_count = 0; \
SYNC_ALL_TO_FRAME(); \
wasm_cluster_thread_waiting_run(exec_env); \
} \
os_mutex_unlock(&exec_env->wait_lock); \
} \
goto *handle_table[*frame_ip++]; \
} while (0)
#else
#define HANDLE_OP_END() FETCH_OPCODE_AND_DISPATCH()
#endif

#else /* else of WASM_ENABLE_LABELS_AS_VALUES */
#define HANDLE_OP(opcode) case opcode:
#if WASM_ENABLE_THREAD_MGR != 0 && WASM_ENABLE_DEBUG_INTERP != 0
#define HANDLE_OP_END() \
do { \
/* Record the current frame_ip, so when exception occurs, \
debugger can know the exact opcode who caused the exception */ \
frame_ip_orig = frame_ip; \
/* Record the current frame_ip, so when exception occurs, \
debugger can know the exact opcode who caused the exception */ \
frame_ip_orig = frame_ip; \
/* Atomic load the exec_env's signal_flag first, and then handle \
more with lock if it is WAMR_SIG_SINGSTEP */ \
GET_SIGNAL_FLAG(); \
if (signal_flag == WAMR_SIG_SINGSTEP) { \
os_mutex_lock(&exec_env->wait_lock); \
while (exec_env->current_status->signal_flag == WAMR_SIG_SINGSTEP \
&& exec_env->current_status->step_count++ == 1) { \
Expand All @@ -1418,25 +1465,8 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
wasm_cluster_thread_waiting_run(exec_env); \
} \
os_mutex_unlock(&exec_env->wait_lock); \
goto *handle_table[*frame_ip++]; \
} while (0)
#else
#define HANDLE_OP_END() FETCH_OPCODE_AND_DISPATCH()
#endif

#else /* else of WASM_ENABLE_LABELS_AS_VALUES */
#define HANDLE_OP(opcode) case opcode:
#if WASM_ENABLE_THREAD_MGR != 0 && WASM_ENABLE_DEBUG_INTERP != 0
#define HANDLE_OP_END() \
os_mutex_lock(&exec_env->wait_lock); \
if (exec_env->current_status->signal_flag == WAMR_SIG_SINGSTEP \
&& exec_env->current_status->step_count++ == 1) { \
exec_env->current_status->step_count = 0; \
SYNC_ALL_TO_FRAME(); \
wasm_cluster_thread_waiting_run(exec_env); \
} \
os_mutex_unlock(&exec_env->wait_lock); \
continue
} \
continue;
#else
#define HANDLE_OP_END() continue
#endif
Expand Down Expand Up @@ -1545,6 +1575,9 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
debug_instance ? &debug_instance->watch_point_list_read : NULL;
bh_list *watch_point_list_write =
debug_instance ? &debug_instance->watch_point_list_write : NULL;
#if WASM_ENABLE_THREAD_MGR != 0
uint32 signal_flag;
#endif
#endif

#if WASM_ENABLE_LABELS_AS_VALUES != 0
Expand Down
6 changes: 3 additions & 3 deletions core/iwasm/libraries/thread-mgr/thread_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,9 @@ wasm_cluster_is_thread_terminated(WASMExecEnv *exec_env);
((signo) == WAMR_SIG_STOP || (signo) == WAMR_SIG_TRAP)

struct WASMCurrentEnvStatus {
uint64 signal_flag : 32;
uint64 step_count : 16;
uint64 running_status : 16;
uint32 signal_flag;
uint16 step_count;
uint16 running_status;
};

WASMCurrentEnvStatus *
Expand Down

0 comments on commit da25906

Please sign in to comment.