Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
36 changes: 27 additions & 9 deletions src/bundler/transpiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,16 +766,24 @@ impl<'a> Transpiler<'a> {
merge_tsconfig_jsx_into(tsconfig, &mut self.options.jsx);
}

let Some(dir) = dir_info.get_entries(self.resolver.generation) else {
// Refresh the listing at our generation, then copy the
// basenames out under `entries_mutex`: concurrent resolvers
// rewrite the `DirEntry` map in place under that lock.
Comment thread
robobun marked this conversation as resolved.
Outdated
if dir_info.get_entries(self.resolver.generation).is_none() {
return Ok(());
}
let dir = {
let _entries_lock = bun_resolver::fs::FileSystem::instance()
.fs
.entries_mutex
.lock_guard();
match dir_info.get_entries_const() {
Some(entries) => DotEnvProbeKeys(
entries.data.iter().map(|(k, _)| Box::from(&**k)).collect(),
),
None => return Ok(()),
}
};
// `get_entries` returns `*mut bun_resolver::fs::DirEntry`
// (BSSMap-owned). `dot_env::Loader::load` takes
// `impl DirEntryProbe` (bun_dotenv sits below `bun_resolver`
// in the crate graph); `bun_resolver::fs::DirEntry` impls it.
// SAFETY: BSSMap singleton owns `*dir`; single-threaded path —
// sole `&mut` for the call.
let dir: &mut bun_resolver::fs::DirEntry = unsafe { &mut *dir };

// `Env.files: Box<[Box<[u8]>]>` but `Loader::load`
// wants `&[&[u8]]`. Re-borrow into a small Vec; the explicit
Expand All @@ -789,7 +797,7 @@ impl<'a> Transpiler<'a> {
} else {
dot_env::DotEnvFileSuffix::Development
};
env.load(dir, &env_files, suffix, skip_default_env)?;
env.load(&dir, &env_files, suffix, skip_default_env)?;
}
DotEnvBehavior::disable => {
env.load_process()?;
Expand All @@ -809,6 +817,16 @@ impl<'a> Transpiler<'a> {
}
}

/// Basenames copied out of a cached `DirEntry` under `entries_mutex` so the
/// dotenv loader can probe them without the lock (see `run_env_loader`).
Comment thread
robobun marked this conversation as resolved.
Outdated
struct DotEnvProbeKeys(Vec<Box<[u8]>>);

impl dot_env::DirEntryProbe for DotEnvProbeKeys {
fn has_comptime_query(&self, query_lower: &'static [u8]) -> bool {
self.0.iter().any(|k| **k == *query_lower)
}
}

// ══════════════════════════════════════════════════════════════════════════
// `ParseResult` / `AlreadyBundled` / `ParseOptions` + `Transpiler::parse*`
// — used by `ModuleLoader::transpile_source_code` (jsc_hooks.rs) and
Expand Down
5 changes: 4 additions & 1 deletion src/jsc/hot_reloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,10 @@ where
// the per-entry mutex.
let _entry_guard = ent.mutex.lock_guard();
ent.set_cache_fd(Fd::INVALID);
ent.need_stat.set(true);
ent.need_stat.store(
true,
core::sync::atomic::Ordering::Release,
);
}
path_string = ent.abs_path;
file_hash = Watcher::get_hash(path_string.as_bytes());
Expand Down
51 changes: 33 additions & 18 deletions src/resolver/dir_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,34 +238,49 @@ impl DirInfo {
}
}

/// Shared-borrow variant of [`get_entries`](Self::get_entries) for the
/// read-only call sites (`.get()`, `.fd`, iteration). The `DirEntry` is a
/// slot in the BSSMap-backed `EntriesOptionMap` singleton (ARENA — process
/// lifetime), so a `&'static` reborrow of the `&'static mut` returned by
/// `entries_at` is sound and needs no `unsafe` here. Prefer this over
/// `get_entries` + per-site raw deref whenever the caller only reads.
pub(crate) fn get_entries_ref(&self, generation: Generation) -> Option<&'static fs::DirEntry> {
/// Shared-borrow variant of [`get_entries`](Self::get_entries) for call
/// sites that already hold `entries_mutex` (the mutex is non-recursive);
/// see [`RealFS::entries_at_locked`](fs::RealFS::entries_at_locked). The
/// `DirEntry` is a slot in the BSSMap-backed `EntriesOptionMap` singleton
/// (ARENA — process lifetime), so a `&'static` reborrow of the
/// `&'static mut` returned by `entries_at_locked` is sound and needs no
/// `unsafe` here. The `.data` map must only be probed/iterated while the
/// lock is held; use [`get_entry`](Self::get_entry) for one-shot lookups.
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn get_entries_ref_locked(
&self,
generation: Generation,
) -> Option<&'static fs::DirEntry> {
let entries_ptr = fs::FileSystem::instance()
.fs
.entries_at(self.entries, generation)?;
.entries_at_locked(self.entries, generation)?;
match entries_ptr {
fs::EntriesOption::Entries(entries) => Some(&**entries),
fs::EntriesOption::Err(_) => None,
}
}

/// [`get_entries_ref`](Self::get_entries_ref) for call sites that already
/// hold `entries_mutex` (the mutex is non-recursive); see
/// [`RealFS::entries_at_locked`](fs::RealFS::entries_at_locked).
pub(crate) fn get_entries_ref_locked(
/// Generation-checked lookup of one basename in this directory's cached
/// listing, performed in a single `entries_mutex` critical section. A
/// concurrent resolver with a newer generation rewrites the `DirEntry` in
/// place under that lock ([`RealFS::entries_at_locked`](fs::RealFS::entries_at_locked)
/// drops the old `data` map's buckets), so probing the map after the lock
/// is released can walk freed buckets. The returned lookup wraps a raw
/// pointer into the process-lifetime `EntryStore`, which stays valid after
/// the lock is dropped.
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn get_entry(
&self,
generation: Generation,
) -> Option<&'static fs::DirEntry> {
let entries_ptr = fs::FileSystem::instance()
.fs
.entries_at_locked(self.entries, generation)?;
match entries_ptr {
fs::EntriesOption::Entries(entries) => Some(&**entries),
query: &[u8],
) -> Option<fs::EntryLookup<'static>> {
let rfs = &mut fs::FileSystem::instance().fs;
// `MutexGuard` stores the mutex by raw pointer, so holding it does not
// keep `rfs` borrowed (same pattern as `RealFS::entries_at`).
Comment thread
robobun marked this conversation as resolved.
Outdated
let _lock = rfs.entries_mutex.lock_guard();
match rfs.entries_at_locked(self.entries, generation)? {
fs::EntriesOption::Entries(entries) => {
let entries: &'static fs::DirEntry = entries;
entries.get(query)
}
fs::EntriesOption::Err(_) => None,
}
}
Expand Down
70 changes: 42 additions & 28 deletions src/resolver/fs.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use core::sync::atomic::{AtomicBool, Ordering};
use std::borrow::Cow;
use std::io::Write as _;

Expand Down Expand Up @@ -136,8 +137,11 @@ impl Default for EntryCache {
// `Entry::symlink` while callers hold a shared
// `&Entry`. `EntryCache` is `Copy`, so `Cell` gives us safe
// `.get()/.set()` through `&self` — the per-entry `mutex` serializes every
// rewrite of these `Cell`s across threads (the `unsafe impl Sync for Entry`
// below opts back in under that external-locking discipline).
// rewrite of these fields across threads (the `unsafe impl Sync for Entry`
// below opts back in under that external-locking discipline). `need_stat`
// is atomic because the `kind()`/`symlink()` fast path reads it without the
// mutex: the Release store after the `cache` write paired with the Acquire
// load is what publishes `cache` to those lock-free readers.
Comment thread
robobun marked this conversation as resolved.
pub struct Entry {
pub(crate) cache: core::cell::Cell<EntryCache>,
pub dir: &'static [u8],
Expand All @@ -148,7 +152,7 @@ pub struct Entry {
pub(crate) base_lowercase_: strings::StringOrTinyString,

pub mutex: Mutex,
pub need_stat: core::cell::Cell<bool>,
pub need_stat: AtomicBool,

pub abs_path: Interned,
}
Expand All @@ -171,13 +175,6 @@ impl Entry {
self.cache.set(c);
}

#[inline(always)]
pub(crate) fn set_cache_kind(&self, kind: EntryKind) {
let mut c = self.cache.get();
c.kind = kind;
self.cache.set(c);
}

#[inline(always)]
pub(crate) fn set_cache_symlink(&self, symlink: Interned) {
let mut c = self.cache.get();
Expand Down Expand Up @@ -226,10 +223,10 @@ impl Entry {
// Generic over `R: EntryKindResolver` so this block is independent of
// which `RealFS` copy `fs` points at (see file-top comment).
pub unsafe fn kind<R: EntryKindResolver>(&self, fs: *mut R, store_fd: bool) -> EntryKind {
if self.need_stat.get() {
if self.need_stat.load(Ordering::Acquire) {
let _guard = self.mutex.lock_guard();
if self.need_stat.get() {
self.need_stat.set(false);
// Relaxed: every write happens under `mutex`, which we hold.
if self.need_stat.load(Ordering::Relaxed) {
// This is technically incorrect, but we are choosing not to handle errors here
// SAFETY: `fs` points at the process-global RealFS singleton; `resolve_kind`
// only does syscalls + string interning, so the short `&mut` cannot alias.
Expand All @@ -240,8 +237,15 @@ impl Entry {
store_fd,
) {
Ok(c) => self.cache.set(c),
Err(_) => return self.cache().kind,
Err(_) => {
self.need_stat.store(false, Ordering::Release);
return self.cache().kind;
}
}
// Clear the flag only after the `cache` write: lock-free readers
// that observe `false` skip the mutex, so this Release store is
// what publishes `cache` to them.
Comment thread
robobun marked this conversation as resolved.
self.need_stat.store(false, Ordering::Release);
}
}
self.cache().kind
Expand All @@ -256,10 +260,10 @@ impl Entry {
fs: *mut R,
store_fd: bool,
) -> &'static [u8] {
if self.need_stat.get() {
if self.need_stat.load(Ordering::Acquire) {
let _guard = self.mutex.lock_guard();
if self.need_stat.get() {
self.need_stat.set(false);
// Relaxed: every write happens under `mutex`, which we hold.
if self.need_stat.load(Ordering::Relaxed) {
// This error can happen if the file was deleted between the time the directory
// was scanned and the time it was read
// SAFETY: see the note on `Entry::kind`.
Expand All @@ -270,8 +274,13 @@ impl Entry {
store_fd,
) {
Ok(c) => self.cache.set(c),
Err(_) => return b"",
Err(_) => {
self.need_stat.store(false, Ordering::Release);
return b"";
}
}
// See `Entry::kind`: Release-publish `cache` before the flag clear.
self.need_stat.store(false, Ordering::Release);
}
}
self.cache().symlink.as_bytes()
Expand Down Expand Up @@ -479,18 +488,23 @@ impl DirEntry {
let _guard = existing.mutex.lock_guard();
existing.dir = self.dir;

existing.need_stat.set(
existing.need_stat.get()
// No cache rewrite here, even when the kind changed: a
// lock-free `kind()`/`symlink()` reader that already
// observed `need_stat == false` reads `cache` without the
// per-entry mutex, so overwriting it would race that read
// (a torn 16-byte `Interned` faults in `as_bytes`).
// Publishing `need_stat = true` instead routes every later
// reader through the mutex, where the lazy stat writes the
// fresh cache; a reader that raced the flag sees the old
// cache, stale but untorn.
// Relaxed load: writes are serialized on the per-entry
// mutex held above.
Comment thread
robobun marked this conversation as resolved.
existing.need_stat.store(
existing.need_stat.load(Ordering::Relaxed)
|| found_kind.is_none()
|| Some(existing.cache().kind) != found_kind,
Ordering::Release,
);
// TODO: is this right?
if Some(existing.cache().kind) != found_kind {
// if found_kind is null, we have set need_stat above, so we
// store an arbitrary kind
existing.set_cache_kind(found_kind.unwrap_or(EntryKind::File));
existing.set_cache_symlink(Interned::EMPTY);
}
break 'brk existing_ptr;
}
}
Expand Down Expand Up @@ -536,7 +550,7 @@ impl DirEntry {
// Call "stat" lazily for performance. The "@material-ui/icons" package
// contains a directory with over 11,000 entries in it and running "stat"
// for each entry was a big performance issue for that package.
addr_of_mut!((*p).need_stat).write(core::cell::Cell::new(found_kind.is_none()));
addr_of_mut!((*p).need_stat).write(AtomicBool::new(found_kind.is_none()));
addr_of_mut!((*p).cache).write(core::cell::Cell::new(EntryCache {
symlink: Interned::EMPTY,
// if found_kind is null, we have set need_stat above, so we
Expand Down
35 changes: 35 additions & 0 deletions src/resolver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,41 @@ pub mod fs {
impl EntriesOption {
// Payload is `&'static mut DirEntry`; auto-deref coerces to `&DirEntry` / `&mut DirEntry`.
bun_core::enum_unwrap!(pub EntriesOption, Entries => fn entries / entries_mut -> DirEntry);

/// Probe the cached listing for `query` and return the lookup plus the
/// listing fd, in one `entries_mutex` critical section. A
/// stale-generation re-read rewrites the slot's `DirEntry` (and frees
/// the old map's buckets) in place under that lock, so the map must
/// not be walked after unlock; the returned entry pointer is
/// EntryStore-owned and stays valid. The caller must NOT already hold
/// `entries_mutex` (it is non-recursive).
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn lookup(
&self,
query: &[u8],
) -> (Option<crate::fs_full::EntryLookup<'static>>, Fd) {
let _lock = FileSystem::instance().fs.entries_mutex.lock_guard();
match self {
EntriesOption::Entries(entries) => {
// SAFETY: ARENA — the `DirEntry` is a process-lifetime
// BSSMap slot (see the enum doc), so widening the local
// reborrow to `'static` is sound.
let entries: &'static DirEntry =
unsafe { &*core::ptr::from_ref::<DirEntry>(&**entries) };
(entries.get(query), entries.fd)
}
EntriesOption::Err(_) => (None, Fd::INVALID),
}
}

/// Listing dir + fd snapshot under `entries_mutex` (watch
/// registration); `None` when the slot holds a read error.
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn dir_and_fd(&self) -> Option<(&'static [u8], Fd)> {
let _lock = FileSystem::instance().fs.entries_mutex.lock_guard();
match self {
EntriesOption::Entries(entries) => Some((entries.dir, entries.fd)),
EntriesOption::Err(_) => None,
}
}
}

// SAFETY: ARENA — `EntriesOption` holds an unbounded `&mut DirEntry` (whose `data`
Expand Down
Loading
Loading