diff --git a/packages/bun-usockets/src/eventing/epoll_kqueue.c b/packages/bun-usockets/src/eventing/epoll_kqueue.c index 1d8ce0f766..4c59ca6382 100644 --- a/packages/bun-usockets/src/eventing/epoll_kqueue.c +++ b/packages/bun-usockets/src/eventing/epoll_kqueue.c @@ -246,8 +246,7 @@ void us_loop_run(struct us_loop_t *loop) { } } -extern int Bun__JSC_onBeforeWait(void*); -extern void Bun__JSC_onAfterWait(void*); +extern void Bun__JSC_onBeforeWait(void * _Nonnull jsc_vm); void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout) { if (loop->num_polls == 0) @@ -264,8 +263,9 @@ void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout /* Emit pre callback */ us_internal_loop_pre(loop); - /* Safe if jsc_vm is NULL */ - int must_call_on_after_wait = Bun__JSC_onBeforeWait(loop->data.jsc_vm); + + if (loop->data.jsc_vm) + Bun__JSC_onBeforeWait(loop->data.jsc_vm); /* Fetch ready polls */ #ifdef LIBUS_USE_EPOLL @@ -276,9 +276,6 @@ void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout } while (IS_EINTR(loop->num_ready_polls)); #endif - 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 28c6d44447..17419ee108 100644 --- a/src/bun.js/bindings/BunJSCEventLoop.cpp +++ b/src/bun.js/bindings/BunJSCEventLoop.cpp @@ -4,83 +4,64 @@ #include #include -// It would be nicer to construct a DropAllLocks in us_loop_run_bun_tick (the only function that -// uses onBeforeWait and onAfterWait), but that code is in C. We use an optional as that lets us -// check whether it's initialized. -static thread_local std::optional drop_all_locks { std::nullopt }; - -extern "C" void WTFTimer__runIfImminent(void* bun_vm); - -// Safe if VM is nullptr -extern "C" int Bun__JSC_onBeforeWait(JSC::VM* vm) +extern "C" void Bun__JSC_onBeforeWait(JSC::VM* _Nonnull vm) { - ASSERT(!drop_all_locks.has_value()); - if (vm) { - bool previouslyHadAccess = vm->heap.hasHeapAccess(); - // sanity check for debug builds to ensure we're not doing a - // use-after-free here - ASSERT(vm->refCount() > 0); - if (previouslyHadAccess) { + ASSERT(vm); + const bool previouslyHadAccess = vm->heap.hasHeapAccess(); + // sanity check for debug builds to ensure we're not doing a + // use-after-free here + ASSERT(vm->refCount() > 0); + if (previouslyHadAccess) { - // 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; + // 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; + 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; - } + // 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; - } + if (remainingRunsUntilSkipReleaseAccess-- > 0) { + // Constellation: + // > If you are not moving a VM to the different thread, then you can aquire the access and do not need to release + vm->heap.stopIfNecessary(); + vm->didEnterVM = false; } } - return 0; -} - -extern "C" void Bun__JSC_onAfterWait(JSC::VM* vm) -{ - if (vm) { - vm->heap.acquireAccess(); - drop_all_locks.reset(); - } }