Merge pull request #371 from svaarala/perf-rework-debugger-skip-interrupt

Fix debugger "lockout" in interrupt handling for better performance

config/config-options/DUK_USE_INTERRUPT_DEBUG_FIXUP.yaml

@@ -0,0 +1,10 @@
+define: DUK_USE_INTERRUPT_DEBUG_FIXUP
+introduced: 1.4.0
+default: false
+tags:
+  - development
+description: >
+  For Duktape development only: enable "interrupt fixup" in call handling
+  so that heap->inst_count_exec and heap->inst_count_interrupt can be
+  manually checked to match.  Only useful when debugging and/or asserts
+  are enabled.

src/duk_debugger.c

@@ -1700,6 +1700,7 @@ DUK_INTERNAL void duk_debug_halt_execution(duk_hthread *thr, duk_bool_t use_prev
 	DUK_ASSERT(thr != NULL);
 	DUK_ASSERT(thr->heap != NULL);
 	DUK_ASSERT(DUK_HEAP_IS_DEBUGGER_ATTACHED(thr->heap));
+	DUK_ASSERT(thr->heap->dbg_processing == 0);
 
 	DUK_HEAP_SET_PAUSED(thr->heap);
 

src/duk_error_misc.c

@@ -74,8 +74,9 @@ DUK_INTERNAL void duk_err_setup_heap_ljstate(duk_hthread *thr, duk_small_int_t l
 #if defined(DUK_USE_DEBUGGER_SUPPORT)
 	/* If something is thrown with the debugger attached and nobody will
 	 * catch it, execution is paused before the longjmp, turning over
-	 * control to the debug client. This allows local state to be examined
-	 * before the stack is unwound.
+	 * control to the debug client.  This allows local state to be examined
+	 * before the stack is unwound.  Errors are not intercepted when debug
+	 * message loop is active (e.g. for Eval).
 	 */
 
 	/* XXX: Allow customizing the pause and notify behavior at runtime
@@ -83,7 +84,9 @@ DUK_INTERNAL void duk_err_setup_heap_ljstate(duk_hthread *thr, duk_small_int_t l
 	 * config options.
 	 */
 #if defined(DUK_USE_DEBUGGER_THROW_NOTIFY) || defined(DUK_USE_DEBUGGER_PAUSE_UNCAUGHT)
-	if (DUK_HEAP_IS_DEBUGGER_ATTACHED(thr->heap) && lj_type == DUK_LJ_TYPE_THROW) {
+	if (DUK_HEAP_IS_DEBUGGER_ATTACHED(thr->heap) &&
+	    !thr->heap->dbg_processing &&
+	    lj_type == DUK_LJ_TYPE_THROW) {
 		duk_context *ctx = (duk_context *) thr;
 		duk_bool_t fatal;
 		duk_hobject *h_obj;

src/duk_js_call.c

@@ -29,7 +29,7 @@ DUK_LOCAL void duk__interrupt_fixup(duk_hthread *thr, duk_hthread *entry_curr_th
 	DUK_ASSERT(thr != NULL);
 	DUK_ASSERT(thr->heap != NULL);
 
-#if 0
+#if defined(DUK_USE_INTERRUPT_DEBUG_FIXUP)
 	if (entry_curr_thread == NULL) {
 		thr->interrupt_init = thr->interrupt_init - thr->interrupt_counter;
 		thr->heap->inst_count_interrupt += thr->interrupt_init;

src/duk_js_executor.c

@@ -1730,9 +1730,19 @@ DUK_LOCAL duk_small_uint_t duk__executor_interrupt(duk_hthread *thr) {
 	/*
 	 *  Avoid nested calls.  Concretely this happens during debugging, e.g.
 	 *  when we eval() an expression.
+	 *
+	 *  Also don't interrupt if we're currently doing debug processing
+	 *  (which can be initiated outside the bytecode executor) as this
+	 *  may cause the debugger to be called recursively.  Check required
+	 *  for correct operation of throw intercept and other "exotic" halting
+	 * scenarios.
 	 */
 
+#if defined(DUK_USE_DEBUGGER_SUPPORT)
+	if (DUK_HEAP_HAS_INTERRUPT_RUNNING(thr->heap) || thr->heap->dbg_processing) {
+#else
 	if (DUK_HEAP_HAS_INTERRUPT_RUNNING(thr->heap)) {
+#endif
 		DUK_DD(DUK_DDPRINT("nested executor interrupt, ignoring"));
 
 		/* Set a high interrupt counter; the original executor
@@ -2271,19 +2281,6 @@ DUK_INTERNAL void duk_js_execute_bytecode(duk_hthread *exec_thr) {
 		 * whenever a thread switch occurs by the DUK_HEAP_SWITCH_THREAD() macro.
 		 */
 #if defined(DUK_USE_INTERRUPT_COUNTER)
-
-#if defined(DUK_USE_DEBUGGER_SUPPORT)
-		if (thr->heap->dbg_processing) {
-			/* Don't interrupt if we're currently doing debug processing as
-			 * this may cause the debugger to be called recursively. Check required
-			 * for correct operation of throw intercept and other "exotic" halting
-			 * scenarios.
-			 */
-
-			goto skip_interrupt;
-		}
-#endif
-
 		int_ctr = thr->interrupt_counter;
 		if (DUK_LIKELY(int_ctr > 0)) {
 			thr->interrupt_counter = int_ctr - 1;
@@ -2318,10 +2315,7 @@ DUK_INTERNAL void duk_js_execute_bytecode(duk_hthread *exec_thr) {
 				goto restart_execution;
 			}
 		}
-
-skip_interrupt:
-
-#endif
+#endif  /* DUK_USE_INTERRUPT_COUNTER */
 #if defined(DUK_USE_INTERRUPT_COUNTER) && defined(DUK_USE_DEBUG)
 		thr->heap->inst_count_exec++;
 #endif