From 21d03fd964b075246af6bd7a7d1a9a0500dce176 Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 11 Mar 2024 12:18:30 +0100 Subject: [PATCH 1/2] drop managed keepalive from thread pool --- ...PortableThreadPool.Browser.Threads.Mono.cs | 2 +- ...dPool.WorkerThread.Browser.Threads.Mono.cs | 12 +--- ...WebWorkerEventLoop.Browser.Threads.Mono.cs | 65 ------------------- src/mono/mono/metadata/icall-decl.h | 3 - src/mono/mono/metadata/icall-def.h | 8 --- src/mono/mono/metadata/threads.c | 37 ----------- 6 files changed, 3 insertions(+), 124 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Browser.Threads.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Browser.Threads.Mono.cs index 632b0c934ee4c0..cc3f606fe6273d 100644 --- a/src/mono/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Browser.Threads.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Browser.Threads.Mono.cs @@ -7,7 +7,7 @@ internal sealed partial class PortableThreadPool { private static partial class WorkerThread { - private static bool IsIOPending => WebWorkerEventLoop.HasJavaScriptInteropDependents; + private static bool IsIOPending => false; } private struct CpuUtilizationReader diff --git a/src/mono/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.Browser.Threads.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.Browser.Threads.Mono.cs index b45dee7fa2fd6f..82645c987d913c 100644 --- a/src/mono/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.Browser.Threads.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.Browser.Threads.Mono.cs @@ -37,7 +37,7 @@ private static partial class WorkerThread private static readonly ThreadStart s_workerThreadStart = WorkerThreadStart; - private sealed record SemaphoreWaitState(PortableThreadPool ThreadPoolInstance, LowLevelLock ThreadAdjustmentLock, WebWorkerEventLoop.KeepaliveToken KeepaliveToken) + private sealed record SemaphoreWaitState(PortableThreadPool ThreadPoolInstance, LowLevelLock ThreadAdjustmentLock) { public bool SpinWait = true; @@ -59,8 +59,7 @@ private static void WorkerThreadStart() } LowLevelLock threadAdjustmentLock = threadPoolInstance._threadAdjustmentLock; - var keepaliveToken = WebWorkerEventLoop.KeepalivePush(); - SemaphoreWaitState state = new(threadPoolInstance, threadAdjustmentLock, keepaliveToken) { SpinWait = true }; + SemaphoreWaitState state = new(threadPoolInstance, threadAdjustmentLock) { SpinWait = true }; // set up the callbacks for semaphore waits, tell // emscripten to keep the thread alive, and return to // the JS event loop. @@ -74,8 +73,6 @@ private static void WorkerThreadStart() private static void WaitForWorkLoop(LowLevelLifoAsyncWaitSemaphore semaphore, SemaphoreWaitState state) { semaphore.PrepareAsyncWait(ThreadPoolThreadTimeoutMs, s_WorkLoopSemaphoreSuccess, s_WorkLoopSemaphoreTimedOut, state); - // thread should still be kept alive - Debug.Assert(state.KeepaliveToken.Valid); } private static void WorkLoopSemaphoreSuccess(LowLevelLifoAsyncWaitSemaphore semaphore, object? stateObject) @@ -91,11 +88,6 @@ private static void WorkLoopSemaphoreTimedOut(LowLevelLifoAsyncWaitSemaphore sem SemaphoreWaitState state = (SemaphoreWaitState)stateObject!; if (ShouldExitWorker(state.ThreadPoolInstance, state.ThreadAdjustmentLock)) { // we're done, kill the thread. - - // we're wrapped in an emscripten eventloop handler which will consult the - // keepalive count, destroy the thread and run the TLS dtor which will - // unregister the thread from Mono - state.KeepaliveToken.Pop(); return; } else { // more work showed up while we were shutting down, go around one more time diff --git a/src/mono/System.Private.CoreLib/src/System/Threading/WebWorkerEventLoop.Browser.Threads.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Threading/WebWorkerEventLoop.Browser.Threads.Mono.cs index 73c2959293d525..367b1df00ee64c 100644 --- a/src/mono/System.Private.CoreLib/src/System/Threading/WebWorkerEventLoop.Browser.Threads.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Threading/WebWorkerEventLoop.Browser.Threads.Mono.cs @@ -12,49 +12,6 @@ namespace System.Threading; /// internal static class WebWorkerEventLoop { - // FIXME: these keepalive calls could be qcalls with a SuppressGCTransitionAttribute - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void KeepalivePushInternal(); - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void KeepalivePopInternal(); - - /// - /// A keepalive token prevents a thread from shutting down even if it returns to the JS event - /// loop. A thread may want a keepalive token if it needs to allow JS code to run to settle JS - /// promises or execute JS timeout callbacks. - /// - internal sealed class KeepaliveToken - { - public bool Valid {get; private set; } - - private KeepaliveToken() { Valid = true; } - - /// - /// Decrement the Emscripten keepalive count. A thread with a zero keepalive count will - /// terminate when it returns from its start function or from an async invocation from the - /// JS event loop. - /// - internal void Pop() { - if (!Valid) - throw new InvalidOperationException(); - Valid = false; - KeepalivePopInternal(); - } - - internal static KeepaliveToken Create() - { - KeepalivePushInternal(); - return new KeepaliveToken(); - } - } - - /// - /// Increment the Emscripten keepalive count. A thread with a positive keepalive can return from its - /// thread start function or a JS event loop invocation and continue running in the JS event - /// loop. - /// - internal static KeepaliveToken KeepalivePush() => KeepaliveToken.Create(); - /// /// Start a thread that may be kept alive on its webworker after the start function returns, /// if the emscripten keepalive count is positive. Once the thread returns to the JS event @@ -73,26 +30,4 @@ internal static void StartExitable(Thread thread, bool captureContext) thread.HasExternalEventLoop = true; thread.UnsafeStart(); } - - /// returns true if the current thread has unsettled JS Interop promises - private static bool HasUnsettledInteropPromises => HasUnsettledInteropPromisesNative(); - - // FIXME: this could be a qcall with a SuppressGCTransitionAttribute - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern bool HasUnsettledInteropPromisesNative(); - - /// returns true if the current WebWorker has JavaScript objects that depend on the - /// current managed thread. - /// - /// If this returns false, the runtime is allowed to allow the current managed thread - /// to exit and for the WebWorker to be recycled by Emscripten for another managed - /// thread. - internal static bool HasJavaScriptInteropDependents - { - // - // FIXME: - // https://github.com/dotnet/runtime/issues/85052 - unsettled promises are not the only relevant - // reasons for keeping a worker thread alive. We will need to add other conditions here. - get => HasUnsettledInteropPromises; - } } diff --git a/src/mono/mono/metadata/icall-decl.h b/src/mono/mono/metadata/icall-decl.h index 1f4ad944a32691..6fd12507ec4905 100644 --- a/src/mono/mono/metadata/icall-decl.h +++ b/src/mono/mono/metadata/icall-decl.h @@ -194,9 +194,6 @@ ICALL_EXPORT void ves_icall_System_Threading_LowLevelLifoSemaphore_ReleaseIn ICALL_EXPORT gpointer ves_icall_System_Threading_LowLevelLifoAsyncWaitSemaphore_InitInternal (void); ICALL_EXPORT void ves_icall_System_Threading_LowLevelLifoAsyncWaitSemaphore_PrepareAsyncWaitInternal (gpointer sem_ptr, gint32 timeout_ms, gpointer success_cb, gpointer timeout_cb, intptr_t user_data); -ICALL_EXPORT MonoBoolean ves_icall_System_Threading_WebWorkerEventLoop_HasUnsettledInteropPromisesNative(void); -ICALL_EXPORT void ves_icall_System_Threading_WebWorkerEventLoop_KeepalivePushInternal (void); -ICALL_EXPORT void ves_icall_System_Threading_WebWorkerEventLoop_KeepalivePopInternal (void); #endif #ifdef TARGET_AMD64 diff --git a/src/mono/mono/metadata/icall-def.h b/src/mono/mono/metadata/icall-def.h index 06f3ab888b729d..c741ccaa447320 100644 --- a/src/mono/mono/metadata/icall-def.h +++ b/src/mono/mono/metadata/icall-def.h @@ -615,14 +615,6 @@ HANDLES(THREAD_10, "SetState", ves_icall_System_Threading_Thread_SetState, void, HANDLES(THREAD_13, "StartInternal", ves_icall_System_Threading_Thread_StartInternal, void, 2, (MonoThreadObject, gint32)) NOHANDLES(ICALL(THREAD_14, "YieldInternal", ves_icall_System_Threading_Thread_YieldInternal)) -/* include these icalls if we're in the threaded wasm runtime, or if we're building a wasm-targeting cross compiler and we need to support --print-icall-table */ -#if (defined(HOST_BROWSER) && !defined(DISABLE_THREADS)) || (defined(TARGET_WASM) && defined(ENABLE_ICALL_SYMBOL_MAP)) -ICALL_TYPE(WEBWORKERLOOP, "System.Threading.WebWorkerEventLoop", WEBWORKERLOOP_1) -NOHANDLES(ICALL(WEBWORKERLOOP_1, "HasUnsettledInteropPromisesNative", ves_icall_System_Threading_WebWorkerEventLoop_HasUnsettledInteropPromisesNative)) -NOHANDLES(ICALL(WEBWORKERLOOP_2, "KeepalivePopInternal", ves_icall_System_Threading_WebWorkerEventLoop_KeepalivePopInternal)) -NOHANDLES(ICALL(WEBWORKERLOOP_3, "KeepalivePushInternal", ves_icall_System_Threading_WebWorkerEventLoop_KeepalivePushInternal)) -#endif - ICALL_TYPE(TYPE, "System.Type", TYPE_1) HANDLES(TYPE_1, "internal_from_handle", ves_icall_System_Type_internal_from_handle, MonoReflectionType, 1, (MonoType_ref)) diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c index 0fe313db637cf9..74f1db84f0518c 100644 --- a/src/mono/mono/metadata/threads.c +++ b/src/mono/mono/metadata/threads.c @@ -4996,26 +4996,6 @@ ves_icall_System_Threading_LowLevelLifoAsyncWaitSemaphore_PrepareAsyncWaitIntern mono_lifo_semaphore_asyncwait_prepare_wait (sem, timeout_ms, (LifoSemaphoreAsyncWaitCallbackFn)success_cb, (LifoSemaphoreAsyncWaitCallbackFn)timedout_cb, user_data); } -void -ves_icall_System_Threading_WebWorkerEventLoop_KeepalivePushInternal (void) -{ - emscripten_runtime_keepalive_push(); -} - -void -ves_icall_System_Threading_WebWorkerEventLoop_KeepalivePopInternal (void) -{ - emscripten_runtime_keepalive_pop(); -} - -extern int mono_wasm_eventloop_has_unsettled_interop_promises(void); - -MonoBoolean -ves_icall_System_Threading_WebWorkerEventLoop_HasUnsettledInteropPromisesNative(void) -{ - return !!mono_wasm_eventloop_has_unsettled_interop_promises(); -} - #endif /* HOST_BROWSER && !DISABLE_THREADS */ /* for the AOT cross compiler with --print-icall-table these don't need to be callable, they just @@ -5033,22 +5013,5 @@ ves_icall_System_Threading_LowLevelLifoAsyncWaitSemaphore_PrepareAsyncWaitIntern g_assert_not_reached (); } -void -ves_icall_System_Threading_WebWorkerEventLoop_KeepalivePushInternal (void) -{ - g_assert_not_reached(); -} - -void -ves_icall_System_Threading_WebWorkerEventLoop_KeepalivePopInternal (void) -{ - g_assert_not_reached(); -} - -MonoBoolean -ves_icall_System_Threading_WebWorkerEventLoop_HasUnsettledInteropPromisesNative(void) -{ - g_assert_not_reached(); -} #endif /* defined(TARGET_WASM) && defined(ENABLE_ICALL_SYMBOL_MAP) */ From 4b4cb6bdd2038917ec7abc56277c18717e25c5d3 Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 11 Mar 2024 12:22:36 +0100 Subject: [PATCH 2/2] more --- src/mono/browser/runtime/cancelable-promise.ts | 2 -- src/mono/browser/runtime/exports-binding.ts | 4 +--- src/mono/browser/runtime/marshal-to-cs.ts | 4 ---- src/mono/browser/runtime/pthreads/index.ts | 1 - .../runtime/pthreads/worker-eventloop.ts | 18 ------------------ 5 files changed, 1 insertion(+), 28 deletions(-) delete mode 100644 src/mono/browser/runtime/pthreads/worker-eventloop.ts diff --git a/src/mono/browser/runtime/cancelable-promise.ts b/src/mono/browser/runtime/cancelable-promise.ts index d55f4052a1a137..cf0b048a2c60cd 100644 --- a/src/mono/browser/runtime/cancelable-promise.ts +++ b/src/mono/browser/runtime/cancelable-promise.ts @@ -9,7 +9,6 @@ import { ControllablePromise, GCHandle, MarshalerToCs } from "./types/internal"; import { ManagedObject } from "./marshal"; import { compareExchangeI32, forceThreadMemoryViewRefresh } from "./memory"; import { mono_log_debug } from "./logging"; -import { settleUnsettledPromise } from "./pthreads"; import { complete_task } from "./managed-exports"; import { marshal_cs_object_to_cs } from "./marshal-to-cs"; @@ -163,7 +162,6 @@ export class PromiseHolder extends ManagedObject { this.isPosted = true; if (WasmEnableThreads) { forceThreadMemoryViewRefresh(); - settleUnsettledPromise(); } // we can unregister the GC handle just on JS side diff --git a/src/mono/browser/runtime/exports-binding.ts b/src/mono/browser/runtime/exports-binding.ts index 986e6f89a9c50a..10d71579ce99d6 100644 --- a/src/mono/browser/runtime/exports-binding.ts +++ b/src/mono/browser/runtime/exports-binding.ts @@ -27,7 +27,7 @@ import { mono_wasm_browser_entropy } from "./crypto"; import { mono_wasm_cancel_promise } from "./cancelable-promise"; import { - mono_wasm_eventloop_has_unsettled_interop_promises, mono_wasm_start_deputy_thread_async, + mono_wasm_start_deputy_thread_async, mono_wasm_pthread_on_pthread_attached, mono_wasm_pthread_on_pthread_unregistered, mono_wasm_pthread_on_pthread_registered, mono_wasm_pthread_set_name, mono_wasm_install_js_worker_interop, mono_wasm_uninstall_js_worker_interop } from "./pthreads"; @@ -44,8 +44,6 @@ export const mono_wasm_threads_imports = !WasmEnableThreads ? [] : [ mono_wasm_pthread_set_name, mono_wasm_start_deputy_thread_async, - // threads.c - mono_wasm_eventloop_has_unsettled_interop_promises, // mono-threads.c mono_wasm_dump_threads, // diagnostics_server.c diff --git a/src/mono/browser/runtime/marshal-to-cs.ts b/src/mono/browser/runtime/marshal-to-cs.ts index 9e82ab8a2711e0..f408e5bebd2b1c 100644 --- a/src/mono/browser/runtime/marshal-to-cs.ts +++ b/src/mono/browser/runtime/marshal-to-cs.ts @@ -22,7 +22,6 @@ import { _zero_region, localHeapViewF64, localHeapViewI32, localHeapViewU8 } fro import { stringToMonoStringRoot, stringToUTF16 } from "./strings"; import { JSMarshalerArgument, JSMarshalerArguments, JSMarshalerType, MarshalerToCs, MarshalerToJs, BoundMarshalerToCs, MarshalerType } from "./types/internal"; import { TypedArray } from "./types/emscripten"; -import { addUnsettledPromise } from "./pthreads"; import { gc_locked } from "./gc-lock"; export const jsinteropDoc = "For more information see https://aka.ms/dotnet-wasm-jsinterop"; @@ -339,9 +338,6 @@ function _marshal_task_to_cs(arg: JSMarshalerArgument, value: Promise, _?: (holder as any)[proxy_debug_symbol] = `PromiseHolder with GCHandle ${gc_handle}`; } - if (WasmEnableThreads) - addUnsettledPromise(); - value.then(data => holder.resolve(data), reason => holder.reject(reason)); } diff --git a/src/mono/browser/runtime/pthreads/index.ts b/src/mono/browser/runtime/pthreads/index.ts index c3128d2be28421..e8a8d4abf48987 100644 --- a/src/mono/browser/runtime/pthreads/index.ts +++ b/src/mono/browser/runtime/pthreads/index.ts @@ -10,7 +10,6 @@ export { populateEmscriptenPool, mono_wasm_init_threads, init_finalizer_thread, waitForThread, replaceEmscriptenPThreadUI } from "./ui-thread"; -export { addUnsettledPromise, settleUnsettledPromise, mono_wasm_eventloop_has_unsettled_interop_promises } from "./worker-eventloop"; export { mono_wasm_pthread_on_pthread_attached, mono_wasm_pthread_on_pthread_unregistered, mono_wasm_pthread_on_pthread_registered, mono_wasm_pthread_set_name, currentWorkerThreadEvents, diff --git a/src/mono/browser/runtime/pthreads/worker-eventloop.ts b/src/mono/browser/runtime/pthreads/worker-eventloop.ts deleted file mode 100644 index 7b8a42b05ac660..00000000000000 --- a/src/mono/browser/runtime/pthreads/worker-eventloop.ts +++ /dev/null @@ -1,18 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -let perThreadUnsettledPromiseCount = 0; - -export function addUnsettledPromise() { - perThreadUnsettledPromiseCount++; -} - -export function settleUnsettledPromise() { - perThreadUnsettledPromiseCount--; -} - -/// Called from the C# threadpool worker loop to find out if there are any -/// unsettled JS promises that need to keep the worker alive -export function mono_wasm_eventloop_has_unsettled_interop_promises(): boolean { - return perThreadUnsettledPromiseCount > 0; -}