Repository navigation
Conversation
|
Status: draft. The pin is the head of oven-sh/mimalloc#57, which is open. Merge order: #41512, then oven-sh/mimalloc#56 and oven-sh/mimalloc#57, then this PR with the pin at the merge commit. The tests run again after the pin moves. How to reproduce the problem bun test test/js/bun/jsc/heapStats-mimalloc.test.ts -t "scavenger thread for every turn"On a release build of main, and on bun 1.4.2, the scavenger thread goes to sleep 391 to 505 times in 500 turns of a 1 ms timer (the bound of the test is 125). With this PR it is 15 to 19 times, and 23 to 26 on a debug build with ASAN. The fix is the mimalloc pin in
CI: the diff is green. Build 122759 has one red test, |
acb4161 to
1fa1c26
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…its schedule The event loop hands the heaps of its thread to mimalloc's scavenger around every poll (us_loop_run_bun_tick). Each hand-off woke the scavenger: a futex wake on the loop thread and a turn of the scavenger, which found that the thread was not due. mimalloc paced the sweeps of a thread (100 ms) and not its wakes. Bump mimalloc to oven-sh/mimalloc#57 (which contains oven-sh/mimalloc#56). The scavenger puts a thread on its schedule when it passes a park of that thread over inside its window, and a park of a thread on the schedule issues no wake. The call in the event loop does not change. - packages/bun-usockets: the comment at the hand-off said that it costs a compare-and-swap per tick. It says what it costs now, and why a tick whose poll cannot block hands off too. - test/js/bun/jsc/heapStats-mimalloc.test.ts: 500 turns of a 1 ms timer put the scavenger thread to sleep 15 to 26 times. It was once for each turn. - bench/snippets/event-loop-turn.mjs: CPU time and sleeps of the threads for one turn of the loop, by the kind of turn. The pin also carries oven-sh/mimalloc#34, which is on bun-dev3-v2.
1fa1c26 to
1ed2410
Compare
This branch has not been deployed
1 participant
Merge after #41512, oven-sh/mimalloc#56 and oven-sh/mimalloc#57, with the pin at the merge commit.
Problem
FUTEX_WAKEcalls and 503 scavenger turns, for 5 sweeps.us_loop_run_bun_tickcallsmi_on_thread_idle_startbefore every poll, which calls_mi_scavenger_wakeeach time (vendor/mimalloc/src/theap.c:381). mimalloc paces a thread's sweeps (100 ms), not its wakes.Fix
test/js/bun/jsc/heapStats-mimalloc.test.ts(fails 6 of 6 with main's pin),gc-controller-cadence.test.tsand 7 leak suites.Background
mi_on_thread_idle_startandmi_on_thread_idle_end, when the scavenger can return the thread's free memory (Return freed memory to the OS on a background thread instead of the JS thread #34181).will_idle_inside_event_loopgate ("mimalloc paces the sweep itself"). A new gate misses a loadedBun.serve, whose polls carry a timeout.Downsides
Bun.servegains no requests per second or p99. Its loop thread blocks twice as often (busy machine).Notes
Findings
mi_on_thread_idle_start, through_mi_scavenger_wake(aFUTEX_WAKEon the loop thread when no wake is pending). The second futex call of a turn is theFUTEX_WAITof the scavenger when it goes back to sleep.mi_on_thread_idle_endmakes none.will_idle_inside_event_loopstops those sweeps and the rest of the wakes in Downsides. It changes nothing for a server: 0 of 1784 polls of a loadedBun.servecarried a zero timeout (syscall trace at 9f70da0). This PR keeps the choice of c094bd9.us_loop_runinlibuv.cdoes not hand off.Bun__JSC_onBeforeWaitsweeps inline, at most once in 100 ms.Builds
Three release builds of one tree of bun (f595f8c, this branch before two edits of a comment and the test). Only the mimalloc checkout differs:
main: eab09015, the pin of main.counters: ec56d2c2, which is os: no address hint for OS allocations above 1 GiB mimalloc#34 and scavenger: count the wakes of parks, the turns of the scavenger and its sweeps of parked threads mimalloc#56. It behaves likemainand counts.this PR: theap: a park wakes the scavenger only while the thread is not on its schedule mimalloc#57 (the tree of 84b81d7d. The pinned 81041984 differs from it in comments only).Linux x64, 16 CPUs of a shared machine with a load average of 250 to 780. Counts are exact. Times come from interleaved rounds, and
countersagainstmainis the noise floor: two builds that behave the same.What mimalloc counted (
Bun.unsafe.mimallocDump(), 5 interleaved runs)Wakes that parks issued / turns of the scavenger / sweeps of a parked thread.
setImmediateturnssetTimeout(fn, 1)fs.promises.statcrypto.pbkdf2Bun.serve, static route, 20000 requests on 16 connectionsBun.serve, handler function, same loadBun.servewith no client for 40 s (2 runs)CPU time of the process for one turn (
bench/snippets/event-loop-turn.mjs, 10 interleaved rounds)Microseconds, median (range). The last two columns are the median of the differences round by round.
How often the scavenger thread went to sleep in those runs (the kernel's count), median (range):
Memory
15 of each 16 buffers of 16 KiB are freed (120 MB), then the loop keeps turning. 8 interleaved runs.
With
MIMALLOC_SCAVENGER=0the last loop keeps 141 to 144 MB. A sweep of a loop that never blocks ends with the poll, before and now.Cost of one call, and size
Instructions of the standalone library (clang -O3, C++,
MI_FREE_USE_PAGEMAP), stepped with gdb from the first instruction of the call to its return, 60 calls each:mi_on_thread_idle_startthat wakesmi_on_thread_idle_starton the schedulemi_on_thread_idle_endsizeof the release binary: text 80676555 (main), 80676683 (counters), 80676955 (this PR). Data (110424) and bss (1822992) do not change.The test
The new test counts how often the scavenger thread goes to sleep in 500 turns of a 1 ms timer. The bound is 125.
src/andpackages/of main cannot show the failure: the fix is the pin inscripts/build/deps/mimalloc.ts.Other suites with the release build of this PR:
spawn-pipe-leak3 pass,fetch-leak31 pass,node-tls-getpeercert-leak2 pass,socket-retention7 pass,net-mongodb-pattern-leak2 pass,zlib/leak8 pass,heapStats-mimalloc9 pass.gc-controller-cadencehas 1 timeout ("the idle collections drop the warmed-up code") andstreams-leakhas 1 ("Absolute memory usage remains relatively constant when reading and writing to a pipe"). Both also time out with the pin of main on this machine.Requests per second and latency
Bun.servewith a handler function, 16 keep-alive connections with one request in flight on each, 4 s, 10 interleaved rounds. The client is the same binary for both builds and runs on 6 other CPUs. The machine was busy (load average 220 to 340). This is not the run on a quiet machine that the review asked for.This PR has more requests per second in 4 of 10 rounds, on 1 CPU and on 4. The differences are inside the spread of the rounds. The loop thread reaches its poll sooner, finds no request waiting more often, and blocks: about twice as often on 1 CPU.
CPU time by thread
One turn of a 1 ms timer, time on a CPU from
/proc/self/task/*/schedstat, 4 interleaved runs of 4000 turns:Most of the CPU time that this PR saves is the scavenger thread's.
What stays
A thread that is due wakes the scavenger for each park until the scavenger finds it parked. A poll that returns at once is over before the scavenger runs, so a loop that never blocks and works between its polls keeps a part of its wakes. Two ways to remove them, neither in this PR: the gate above, or a short schedule in mimalloc for a thread that the scavenger cannot find parked.
The pin
eab09015 to 81041984 is 4 commits: oven-sh/mimalloc#34 (7b4eb150 and its merge 3a13a5ff, already on
bun-dev3-v2, what #41512 bumps to), oven-sh/mimalloc#56 (counters) and oven-sh/mimalloc#57. Thecountersbuild has the first two, so the tables show oven-sh/mimalloc#57 alone. After the two PRs merge, the pin moves to the merge commit and the tests run again.Related
epoll_kqueue.c. Its IOCP loop hands off only on a tick that parks, because "the hand-off wakes the scavenger thread (a syscall)". With this pin most hand-offs issue no wake.Self-review
Two reviews: the first raised 8 concerns, the second 9.
ThreadPool.rsand Remove libuv on Windows #42819 do not follow that. The header now states the cost and the precondition only, and the Rust comment is the one of main.subproc->paced_count, which the review called optional. The end of a thread takes its mark off on a thread that is not the scavenger, so the count is shared. It costs 8 bytes and one load for each turn of the scavenger.Not measured
strace,perfandvalgrindare not in the container. The counts come from mimalloc's counters, the kernel's per-thread context switch counts and gdb. Not run here: macOS, Windows, aarch64, musl. The CI of the fork passed 23 of 23 jobs at 81041984 (Linux x64 and arm64 with ASAN, UBSAN and TSAN, macOS, Windows, FreeBSD).no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/jsc/heapStats-mimalloc.test.ts