Skip to content

Commit

Permalink
[browser][mt] drop managed keepalive from thread pool (#99524)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelsavara authored Mar 12, 2024
1 parent 8d952f7 commit 7a7d8d5
Show file tree
Hide file tree
Showing 11 changed files with 4 additions and 152 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
Expand All @@ -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)
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,49 +12,6 @@ namespace System.Threading;
/// </summary>
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();

/// <summary>
/// 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.
/// </summary>
internal sealed class KeepaliveToken
{
public bool Valid {get; private set; }

private KeepaliveToken() { Valid = true; }

/// <summary>
/// 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.
/// </summary>
internal void Pop() {
if (!Valid)
throw new InvalidOperationException();
Valid = false;
KeepalivePopInternal();
}

internal static KeepaliveToken Create()
{
KeepalivePushInternal();
return new KeepaliveToken();
}
}

/// <summary>
/// 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.
/// </summary>
internal static KeepaliveToken KeepalivePush() => KeepaliveToken.Create();

/// <summary>
/// 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
Expand All @@ -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();

/// <summary>returns true if the current WebWorker has JavaScript objects that depend on the
/// current managed thread.</summary>
///
/// <remarks>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.</remarks>
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;
}
}
2 changes: 0 additions & 2 deletions src/mono/browser/runtime/cancelable-promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/mono/browser/runtime/exports-binding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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
Expand Down
4 changes: 0 additions & 4 deletions src/mono/browser/runtime/marshal-to-cs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -339,9 +338,6 @@ function _marshal_task_to_cs(arg: JSMarshalerArgument, value: Promise<any>, _?:
(holder as any)[proxy_debug_symbol] = `PromiseHolder with GCHandle ${gc_handle}`;
}

if (WasmEnableThreads)
addUnsettledPromise();

value.then(data => holder.resolve(data), reason => holder.reject(reason));
}

Expand Down
1 change: 0 additions & 1 deletion src/mono/browser/runtime/pthreads/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 0 additions & 18 deletions src/mono/browser/runtime/pthreads/worker-eventloop.ts

This file was deleted.

3 changes: 0 additions & 3 deletions src/mono/mono/metadata/icall-decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions src/mono/mono/metadata/icall-def.h
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
37 changes: 0 additions & 37 deletions src/mono/mono/metadata/threads.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) */

0 comments on commit 7a7d8d5

Please sign in to comment.