diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index 1c5082acfb23..1c29ceba5b15 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -87,9 +87,8 @@ new!(pub BUN_FEATURE_FLAG_DUMP_CODE: string, "BUN_FEATURE_FLAG_DUMP_CODE", {}); new!(pub BUN_GC_RUNS_UNTIL_SKIP_RELEASE_ACCESS: unsigned, "BUN_GC_RUNS_UNTIL_SKIP_RELEASE_ACCESS", {}); new!(pub BUN_GC_TIMER_DISABLE: boolean, "BUN_GC_TIMER_DISABLE", {}); new!(pub BUN_GC_TIMER_INTERVAL: unsigned, "BUN_GC_TIMER_INTERVAL", {}); -// TODO(markovejnovic): It's unclear why the default here is 100_000, but this was legacy behavior -// so we'll keep it for now. -new!(pub BUN_INOTIFY_COALESCE_INTERVAL: unsigned, "BUN_INOTIFY_COALESCE_INTERVAL", { default: 100_000 }); +// Nanoseconds. Honoured by every watcher backend, not just inotify. +new!(pub BUN_INOTIFY_COALESCE_INTERVAL: unsigned, "BUN_INOTIFY_COALESCE_INTERVAL", { default: 10_000_000 }); new!(pub BUN_INSPECT: string, "BUN_INSPECT", { default: b"" }); new!(pub BUN_INSPECT_CONNECT_TO: string, "BUN_INSPECT_CONNECT_TO", { default: b"" }); new!(pub BUN_INSPECT_PRELOAD: string, "BUN_INSPECT_PRELOAD", {}); diff --git a/src/jsc/hot_reloader.rs b/src/jsc/hot_reloader.rs index b94eb1050993..af09acf185b0 100644 --- a/src/jsc/hot_reloader.rs +++ b/src/jsc/hot_reloader.rs @@ -540,6 +540,11 @@ where } pub(crate) fn append(&mut self, id: u32) { + // One save repeats its hash; duplicates would hit the mid-update `enqueue` below. + if self.hashes[..self.count as usize].contains(&id) { + return; + } + if self.count == 8 { self.enqueue(); self.count = 0; diff --git a/src/watcher/INotifyWatcher.rs b/src/watcher/INotifyWatcher.rs index 23637c21ca76..adf5bba80299 100644 --- a/src/watcher/INotifyWatcher.rs +++ b/src/watcher/INotifyWatcher.rs @@ -5,12 +5,15 @@ use core::ffi::c_int; use core::mem::{align_of, size_of}; use core::sync::atomic::{AtomicU32, Ordering}; -use bun_core::{ZStr, env_var, output as Output}; +use bun_core::{ZStr, output as Output}; use bun_paths::MAX_PATH_BYTES; use bun_sys::{self, Fd}; use bun_threading::Futex; -use crate::watcher_impl::{MAX_COUNT as max_count, Op, WatchEvent, WatchItemIndex, Watcher}; +use crate::watcher_impl::{ + MAX_COALESCE_ITERATIONS, MAX_COUNT as max_count, Op, WatchEvent, WatchItemIndex, Watcher, + coalesce_interval_ns, coalesce_timespec, +}; use bun_collections::index_sort; bun_core::declare_scope!(watcher, visible); @@ -54,8 +57,8 @@ pub struct INotifyWatcher { read_ptr: Option, pub(crate) watch_count: AtomicU32, - /// nanoseconds - pub(crate) coalesce_interval: isize, + /// See [`coalesce_interval_ns`]. + pub(crate) coalesce_interval: u64, } impl Default for INotifyWatcher { @@ -67,7 +70,7 @@ impl Default for INotifyWatcher { eventlist_ptrs: [core::ptr::null(); max_count], read_ptr: None, watch_count: AtomicU32::new(0), - coalesce_interval: 100_000, + coalesce_interval: 0, } } } @@ -197,10 +200,7 @@ impl INotifyWatcher { Ok(Self { fd, loaded: true, - coalesce_interval: env_var::BUN_INOTIFY_COALESCE_INTERVAL - .get() - .and_then(|v| isize::try_from(v).ok()) - .unwrap_or(100_000), + coalesce_interval: coalesce_interval_ns(), ..Self::default() }) } @@ -246,19 +246,22 @@ impl INotifyWatcher { return Ok(&[]); } - // IN_MODIFY is very noisy - // we do a 0.1ms sleep to try to coalesce events better - const DOUBLE_READ_THRESHOLD: usize = Event::LARGEST_SIZE * (max_count / 2); - if read_len < DOUBLE_READ_THRESHOLD { + // Drain until quiet; beyond `max_count` the parser sets `read_ptr` anyway. + let timespec = coalesce_timespec(self.coalesce_interval); + let mut iterations: u32 = 0; + while read_len < size_of::() * max_count + && iterations < MAX_COALESCE_ITERATIONS + { + let rest = &mut self.eventlist_bytes.0[read_len..]; + if rest.len() < Event::LARGEST_SIZE { + break; // buffer nearly full + } + let mut fds = [system::pollfd { fd: self.fd.native(), events: (libc::POLLIN | libc::POLLERR) as _, revents: 0, }]; - let timespec = libc::timespec { - tv_sec: 0, - tv_nsec: self.coalesce_interval as _, - }; // SAFETY: fds and timespec are valid stack locals; sigmask is null. let poll_n = unsafe { system::ppoll( @@ -268,37 +271,34 @@ impl INotifyWatcher { core::ptr::null(), ) }; - if poll_n > 0 { - 'inner: loop { - let rest = &mut self.eventlist_bytes.0[read_len..]; - debug_assert!(!rest.is_empty()); - // SAFETY: fd valid; rest is a valid mutable buffer. - let new_rc = unsafe { - system::read( - self.fd.native(), - rest.as_mut_ptr(), - rest.len(), - ) - }; - let e = get_errno(new_rc); - match e { - E::SUCCESS => { - read_len += usize::try_from(new_rc).expect("int cast"); - break 'outer read_len; - } - E::EAGAIN | E::EINTR => { - continue 'inner; - } - _ => { - return Err(bun_sys::Error { - errno: e as u32 as _, - syscall: bun_sys::Tag::read, - ..Default::default() - }); - } + if poll_n <= 0 { + break; // quiet + } + + 'inner: loop { + // SAFETY: fd valid; rest is a valid mutable buffer. + let new_rc = unsafe { + system::read(self.fd.native(), rest.as_mut_ptr(), rest.len()) + }; + let e = get_errno(new_rc); + match e { + E::SUCCESS => { + read_len += usize::try_from(new_rc).expect("int cast"); + break 'inner; + } + E::EAGAIN | E::EINTR => { + continue 'inner; + } + _ => { + return Err(bun_sys::Error { + errno: e as u32 as _, + syscall: bun_sys::Tag::read, + ..Default::default() + }); } } } + iterations += 1; } break 'outer read_len; diff --git a/src/watcher/KEventWatcher.rs b/src/watcher/KEventWatcher.rs index c784b8636584..8ac36542ae19 100644 --- a/src/watcher/KEventWatcher.rs +++ b/src/watcher/KEventWatcher.rs @@ -1,12 +1,16 @@ use bun_core::output as Output; use bun_sys::Fd; -use crate::watcher_impl::{Op, WatchEvent, Watcher}; +use crate::watcher_impl::{ + MAX_COALESCE_ITERATIONS, Op, WatchEvent, Watcher, coalesce_interval_ns, coalesce_timespec, +}; pub(crate) type Platform = KEventWatcher; pub struct KEventWatcher { pub(crate) fd: Fd, + /// See [`coalesce_interval_ns`]. + pub(crate) coalesce_interval: u64, } const CHANGELIST_COUNT: usize = 128; @@ -17,7 +21,10 @@ impl KEventWatcher { if fd.native() == 0 { return Err(crate::Error::KQueueError); } - Ok(Self { fd }) + Ok(Self { + fd, + coalesce_interval: coalesce_interval_ns(), + }) } pub(crate) fn stop(&mut self) { @@ -58,13 +65,16 @@ pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> { let mut count = bun_sys::kevent(fd, &[], &mut changelist, None)?; - // Give the events more time to coalesce - if count < CHANGELIST_COUNT / 2 { - let ts = libc::timespec { - tv_sec: 0, - tv_nsec: 100_000, - }; // 0.0001 seconds - count += bun_sys::kevent(fd, &[], &mut changelist[count..], Some(&ts))?; + // Drain until quiet. + let ts = coalesce_timespec(this.platform.coalesce_interval); + let mut iterations: u32 = 0; + while count > 0 && count < CHANGELIST_COUNT && iterations < MAX_COALESCE_ITERATIONS { + // Don't let a failed drain poll discard the events already read. + match bun_sys::kevent(fd, &[], &mut changelist[count..], Some(&ts)) { + Ok(0) | Err(_) => break, + Ok(extra) => count += extra, + } + iterations += 1; } let changes = &changelist[..count]; diff --git a/src/watcher/Watcher.rs b/src/watcher/Watcher.rs index a66df8e900ea..eac12a606766 100644 --- a/src/watcher/Watcher.rs +++ b/src/watcher/Watcher.rs @@ -4,7 +4,7 @@ use core::fmt; use std::borrow::Cow; use bun_collections::MultiArrayList; -use bun_core::{ThreadLock, ZStr, feature_flags, output as Output, strings, zstr}; +use bun_core::{ThreadLock, ZStr, env_var, feature_flags, output as Output, strings, zstr}; use bun_sys::{self as sys, Fd}; use bun_threading::Mutex; @@ -28,6 +28,26 @@ bun_core::define_scoped_log!(log, watcher, visible); pub const MAX_COUNT: usize = 128; +/// Quiet window (ns) that folds one editor save's several events into one dispatch (#13511). +pub(crate) fn coalesce_interval_ns() -> u64 { + env_var::BUN_INOTIFY_COALESCE_INTERVAL + .get() + .expect("BUN_INOTIFY_COALESCE_INTERVAL declares a default") +} + +/// Caps a drain (kqueue wakes once per event) so a continuously written file can't starve the loop. +pub(crate) const MAX_COALESCE_ITERATIONS: u32 = 32; + +/// `ns` split into a `timespec`; `tv_nsec` must stay below one second. +#[cfg(not(windows))] +pub(crate) fn coalesce_timespec(ns: u64) -> libc::timespec { + const NS_PER_S: u64 = 1_000_000_000; + libc::timespec { + tv_sec: (ns / NS_PER_S) as _, + tv_nsec: (ns % NS_PER_S) as _, + } +} + #[cfg(any(target_os = "macos", target_os = "freebsd"))] pub const REQUIRES_FILE_DESCRIPTORS: bool = true; #[cfg(not(any(target_os = "macos", target_os = "freebsd")))] diff --git a/src/watcher/WindowsWatcher.rs b/src/watcher/WindowsWatcher.rs index edd5035fb7e7..6fc89667e545 100644 --- a/src/watcher/WindowsWatcher.rs +++ b/src/watcher/WindowsWatcher.rs @@ -3,7 +3,10 @@ use core::mem::size_of; use core::ptr; -use crate::watcher_impl::{Op, WatchEvent, WatchItemColumns, WatchItemIndex, Watcher}; +use crate::watcher_impl::{ + MAX_COALESCE_ITERATIONS, Op, WatchEvent, WatchItemColumns, WatchItemIndex, Watcher, + coalesce_interval_ns, +}; use bun_core::strings; use bun_paths::resolve_path::{ParentEqual, is_parent_or_equal}; use bun_paths::{PathBuffer, WPathBuffer}; @@ -22,6 +25,8 @@ pub struct WindowsWatcher { pub(crate) watcher: DirWatcher, pub(crate) buf: PathBuffer, pub(crate) base_idx: usize, + /// [`coalesce_interval_ns`] in the milliseconds `GetQueuedCompletionStatus` takes. + pub(crate) coalesce_interval_ms: w::DWORD, } impl Default for WindowsWatcher { @@ -35,6 +40,7 @@ impl Default for WindowsWatcher { }, buf: PathBuffer::uninit(), base_idx: 0, + coalesce_interval_ms: 0, } } } @@ -293,14 +299,18 @@ impl WindowsWatcher { root.len() }; + // div_ceil so a sub-millisecond override still waits instead of becoming 0. + self.coalesce_interval_ms = + w::DWORD::try_from(coalesce_interval_ns().div_ceil(1_000_000)).unwrap_or(w::INFINITE); + // disarm the cleanup scopeguards on success scopeguard::ScopeGuard::into_inner(iocp_guard); scopeguard::ScopeGuard::into_inner(handle_guard); Ok(()) } - /// wait until new events are available - fn next(&mut self, timeout: Timeout) -> bun_sys::Result> { + /// Waits up to `timeout_ms` (a `GetQueuedCompletionStatus` timeout) for events. + fn next(&mut self, timeout_ms: w::DWORD) -> bun_sys::Result> { if let Err(err) = self.watcher.prepare() { bun_core::scoped_log!(watcher, "prepare() returned error"); return Err(err); @@ -317,7 +327,7 @@ impl WindowsWatcher { &mut nbytes, &mut key, &mut overlapped, - timeout as w::DWORD, + timeout_ms, ) }; if rc == 0 { @@ -389,13 +399,6 @@ impl WindowsWatcher { } } -#[repr(u32)] -#[derive(Copy, Clone, Eq, PartialEq)] -pub(crate) enum Timeout { - Infinite = w::INFINITE, - None = 0, -} - pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> { // We re-borrow buf inside the inner loop instead of holding `&this.platform.buf` // across calls to `this.platform.next()`. @@ -403,17 +406,16 @@ pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> { let mut event_id: usize = 0; - // first wait has infinite timeout - we're waiting for the next event and don't want to spin - let mut timeout = Timeout::Infinite; - loop { - let mut iter = match this.platform.next(timeout)? { + // `<=`: the blocking INFINITE wait is iteration zero; the coalesce sweeps are the rest. + let mut timeout_ms: w::DWORD = w::INFINITE; + let mut iterations: u32 = 0; + while iterations <= MAX_COALESCE_ITERATIONS { + let mut iter = match this.platform.next(timeout_ms)? { Some(it) => it, None => break, }; - // after the first wait, we want to coalesce further events but don't want to wait for them - // NOTE: using a 1ms timeout would be ideal, but that actually makes the thread wait for at least 10ms more than it should - // Instead we use a 0ms timeout, which may not do as much coalescing but is more responsive. - timeout = Timeout::None; + timeout_ms = this.platform.coalesce_interval_ms; + iterations += 1; bun_core::scoped_log!( watcher, "number of watched items: {}", diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 8ab6f31dd9e6..9c13f0694cee 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -1,7 +1,18 @@ import { spawn } from "bun"; import { beforeEach, expect, it } from "bun:test"; -import { copyFileSync, cpSync, readFileSync, renameSync, rmSync, unlinkSync, writeFileSync } from "fs"; -import { bunEnv, bunExe, isDebug, isWindows, tmpdirSync, waitForFileToExist } from "harness"; +import { + closeSync, + copyFileSync, + cpSync, + openSync, + readFileSync, + renameSync, + rmSync, + unlinkSync, + writeFileSync, + writeSync, +} from "fs"; +import { bunEnv, bunExe, isDebug, isIntelMacOS, isWindows, tmpdirSync, waitForFileToExist } from "harness"; import { join } from "path"; const timeout = isDebug ? Infinity : 10_000; @@ -332,6 +343,100 @@ it( timeout, ); +// Intel macOS runners stretch `sleepSync(2)` past the 10 ms coalesce window, +// splitting the burst; the bug (#13511) was reported on Linux and Windows. +it.skipIf(isIntelMacOS)( + "coalesces a burst of writes into a single reload", + async () => { + // https://github.com/oven-sh/bun/issues/13511 + // + // A fresh directory (not `cwd`, which holds the whole fixture set) so the + // directory watch only sees the file under test. + const dir = tmpdirSync(); + const root = join(dir, "coalesce.js"); + // `globalThis.count` survives a hot reload, so it counts evaluations. + // `console.write` so the line is written atomically (see hot-runner.js). + const body = `globalThis.count = (globalThis.count || 0) + 1; +console.write("[eval] " + globalThis.count + "\\n"); +setInterval(() => {}, 1e6); +`; + writeFileSync(root, body); + + await using runner = spawn({ + cmd: [bunExe(), "--hot", "run", root], + env: bunEnv, + cwd: dir, + stdout: "pipe", + stderr: "inherit", + stdin: "ignore", + }); + + const evals: number[] = []; + let buffered = ""; + (async () => { + for await (const chunk of runner.stdout) { + buffered += new TextDecoder().decode(chunk); + let nl: number; + while ((nl = buffered.indexOf("\n")) !== -1) { + const line = buffered.slice(0, nl); + buffered = buffered.slice(nl + 1); + const m = line.match(/\[eval\] (\d+)/); + if (m) evals.push(Number(m[1])); + } + } + })().catch(() => {}); + + while (evals.length < 1) await Bun.sleep(1); + + // An editor-save-shaped burst: each write emits an event on the file and + // directory watches, and the 2 ms gaps let the watcher thread observe + // them mid-burst while staying inside the 10 ms coalesce window. + // + // The gaps are measured: a loaded runner can stretch `sleepSync(2)` past + // the window, and writes that far apart are legitimately separate saves. + // A burst that was both stretched and split proves nothing either way, so + // it is retried; a split burst only counts against the watcher when its + // gaps stayed well inside the window. + const maxTightGapMs = 8; + let trial: { reloads: number; gapsMs: number[] } | undefined; + for (let attempt = 0; attempt < 8 && trial === undefined; attempt++) { + const evalsBefore = evals.length; + const gapsMs: number[] = []; + const fd = openSync(root, "a"); + try { + let last = performance.now(); + for (let i = 0; i < 10; i++) { + writeSync(fd, "\n"); + Bun.sleepSync(2); + const now = performance.now(); + gapsMs.push(now - last); + last = now; + } + } finally { + closeSync(fd); + } + + while (evals.length === evalsBefore) await Bun.sleep(1); + // Give any extra reloads time to surface (same settle as the "random + // file" test below). + await Bun.sleep(200); + + const reloads = evals.length - evalsBefore; + if (reloads === 1 || Math.max(...gapsMs) <= maxTightGapMs) { + trial = { reloads, gapsMs }; + } + } + + runner.kill(); + + // One reload for the whole burst. `gapsMs` is carried along so a failure + // shows how tight the burst actually was. + expect(trial).toBeDefined(); + expect(trial).toEqual({ ...trial!, reloads: 1 }); + }, + timeout, +); + it( "should not hot reload when a random file is written", async () => { @@ -600,7 +705,10 @@ ${Buffer.alloc(counter * 2, " ").toString()}throw new Error('${counter}');`, writeFull(0); await using runner = spawn({ cmd: [bunExe(), "--smol", "--hot", "run", hotRunnerRoot], - env: bunEnv, + // The race under test needs the self-write's event dispatched before + // the rejection is reported; the default 10 ms coalesce window would + // close it. + env: { ...bunEnv, BUN_INOTIFY_COALESCE_INTERVAL: "0" }, cwd, stdout: "ignore", stderr: "pipe",