Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[browser][mt] drop managed keepalive from thread pool #99524

Merged
merged 3 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) */

Loading