diff --git a/packages/bun-usockets/src/eventing/epoll_kqueue.c b/packages/bun-usockets/src/eventing/epoll_kqueue.c index 3b6c7e438f..1d8ce0f766 100644 --- a/packages/bun-usockets/src/eventing/epoll_kqueue.c +++ b/packages/bun-usockets/src/eventing/epoll_kqueue.c @@ -246,7 +246,7 @@ void us_loop_run(struct us_loop_t *loop) { } } -extern void Bun__JSC_onBeforeWait(void*); +extern int Bun__JSC_onBeforeWait(void*); extern void Bun__JSC_onAfterWait(void*); void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout) { @@ -265,7 +265,7 @@ void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout us_internal_loop_pre(loop); /* Safe if jsc_vm is NULL */ - Bun__JSC_onBeforeWait(loop->data.jsc_vm); + int must_call_on_after_wait = Bun__JSC_onBeforeWait(loop->data.jsc_vm); /* Fetch ready polls */ #ifdef LIBUS_USE_EPOLL @@ -276,7 +276,9 @@ void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout } while (IS_EINTR(loop->num_ready_polls)); #endif - Bun__JSC_onAfterWait(loop->data.jsc_vm); + if (must_call_on_after_wait) { + Bun__JSC_onAfterWait(loop->data.jsc_vm); + } /* Iterate ready polls, dispatching them by type */ for (loop->current_ready_poll = 0; loop->current_ready_poll < loop->num_ready_polls; loop->current_ready_poll++) { diff --git a/src/bun.js/bindings/BunJSCEventLoop.cpp b/src/bun.js/bindings/BunJSCEventLoop.cpp index 9fafa9f6c2..28c6d44447 100644 --- a/src/bun.js/bindings/BunJSCEventLoop.cpp +++ b/src/bun.js/bindings/BunJSCEventLoop.cpp @@ -12,7 +12,7 @@ static thread_local std::optional drop_all_locks { st extern "C" void WTFTimer__runIfImminent(void* bun_vm); // Safe if VM is nullptr -extern "C" void Bun__JSC_onBeforeWait(JSC::VM* vm) +extern "C" int Bun__JSC_onBeforeWait(JSC::VM* vm) { ASSERT(!drop_all_locks.has_value()); if (vm) { @@ -20,11 +20,61 @@ extern "C" void Bun__JSC_onBeforeWait(JSC::VM* vm) // sanity check for debug builds to ensure we're not doing a // use-after-free here ASSERT(vm->refCount() > 0); - drop_all_locks.emplace(*vm); if (previouslyHadAccess) { - vm->heap.releaseAccess(); + + // Releasing heap access is a balance between: + // 1. CPU usage + // 2. Memory usage + // + // Not releasing heap access causes benchmarks like + // https://github.com/oven-sh/bun/pull/14885 to regress due to + // finalizers not being called quickly enough. + // + // Releasing heap access too often causes high idle CPU usage. + // + // For the following code: + // ``` + // setTimeout(() => {}, 10 * 1000) + // ``` + // + // command time -v when with defaultRemainingRunsUntilSkipReleaseAccess = 0: + // + // Involuntary context switches: 605 + // + // command time -v when with defaultRemainingRunsUntilSkipReleaseAccess = 5: + // + // Involuntary context switches: 350 + // + // command time -v when with defaultRemainingRunsUntilSkipReleaseAccess = 10: + // + // Involuntary context switches: 241 + // + // Also comapre the #14885 benchmark with different values. + // + // The idea here is if you entered JS "recently", running any + // finalizers that might've been waiting to be run is a good idea. + // But if you haven't, like if the process is just waiting on I/O + // then don't bother. + static constexpr int defaultRemainingRunsUntilSkipReleaseAccess = 10; + + static thread_local int remainingRunsUntilSkipReleaseAccess = 0; + + // Note: usage of `didEnterVM` in JSC::VM conflicts with Options::validateDFGClobberize + // We don't need to use that option, so it should be fine. + if (vm->didEnterVM) { + vm->didEnterVM = false; + remainingRunsUntilSkipReleaseAccess = defaultRemainingRunsUntilSkipReleaseAccess; + } + + if (remainingRunsUntilSkipReleaseAccess-- > 0) { + drop_all_locks.emplace(*vm); + vm->heap.releaseAccess(); + vm->didEnterVM = false; + return 1; + } } } + return 0; } extern "C" void Bun__JSC_onAfterWait(JSC::VM* vm)