Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
100 changes: 64 additions & 36 deletions src/bun_alloc/bun_alloc.zig
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ pub const Result = struct {
status: ItemStatus,

pub fn hasCheckedIfExists(r: *const Result) bool {
return r.index.index != Unassigned.index;
// A reclaimed slot (status == .unknown but index is a real slot) must be
// treated as "never looked up" so callers re-resolve instead of returning
// stale data. See BSSMap.remove / BSSMap.getOrPut.
return r.status != .unknown;
Comment thread
robobun marked this conversation as resolved.
}

pub fn isOverflowing(r: *const Result, comptime count: usize) bool {
Expand Down Expand Up @@ -526,6 +529,11 @@ pub fn BSSMap(comptime ValueType: type, comptime count: anytype, comptime store_
mutex: Mutex = .{},
backing_buf: [count]ValueType,
backing_buf_used: u16,
/// Slots freed by `remove()` that can be reused by the next `put()` for
/// the same key. Without this, every remove+getOrPut+put cycle consumes
/// a fresh `backing_buf` slot (and eventually overflows to the heap),
/// leaking the old slot and whatever heap pointers it owned.
reclaimable: std.AutoHashMapUnmanaged(HashKeyType, IndexType),

pub var instance: *Self = undefined;

Expand All @@ -542,6 +550,7 @@ pub fn BSSMap(comptime ValueType: type, comptime count: anytype, comptime store_
instance.overflow_list.zero();
instance.backing_buf_used = 0;
instance.mutex = .{};
instance.reclaimable = .{};
loaded = true;
}

Expand All @@ -550,6 +559,7 @@ pub fn BSSMap(comptime ValueType: type, comptime count: anytype, comptime store_

pub fn deinit(self: *Self) void {
self.index.deinit(self.allocator);
self.reclaimable.deinit(self.allocator);
bun.default_allocator.destroy(instance);
loaded = false;
}
Expand All @@ -567,17 +577,25 @@ pub fn BSSMap(comptime ValueType: type, comptime count: anytype, comptime store_
const index = try self.index.getOrPut(self.allocator, _key);

if (index.found_existing) {
switch (index.value_ptr.index) {
NotFound.index => return Result{ .hash = _key, .index = index.value_ptr.*, .status = .not_found },
Unassigned.index => {},
else => return Result{ .hash = _key, .index = index.value_ptr.*, .status = .exists },
}
} else {
index.value_ptr.* = Unassigned;
}

// status is .unknown — if this key previously had a real slot that was
// invalidated via remove(), hand it back so put() overwrites in place
// instead of burning a fresh slot.
if (self.reclaimable.get(_key)) |slot| {
return Result{
.hash = _key,
.index = index.value_ptr.*,
.status = switch (index.value_ptr.index) {
NotFound.index => .not_found,
Unassigned.index => .unknown,
else => .exists,
},
.index = slot,
.status = .unknown,
};
}
index.value_ptr.* = Unassigned;

return Result{
.hash = _key,
Expand All @@ -599,6 +617,10 @@ pub fn BSSMap(comptime ValueType: type, comptime count: anytype, comptime store_
self.mutex.lock();
defer self.mutex.unlock();

// Leave any reclaimable slot in place: getOrPut early-returns on
// NotFound before consulting reclaimable, so keeping the entry lets
// a delete→recreate→bust cycle reuse the original slot instead of
// permanently orphaning it.
self.index.put(self.allocator, result.hash, NotFound) catch unreachable;
}
Comment thread
claude[bot] marked this conversation as resolved.

Expand All @@ -616,13 +638,24 @@ pub fn BSSMap(comptime ValueType: type, comptime count: anytype, comptime store_
self.mutex.lock();
defer self.mutex.unlock();

// Consume any parked slot for this key up front. If the caller's
// result came from getOrPut() it already carries the reclaimed
// index, but getOrPut() early-returns on NotFound without
// consulting reclaimable — so a put() after markNotFound() would
// otherwise burn a fresh slot while the parked one is discarded.
const reclaimed = self.reclaimable.fetchRemove(result.hash);

if (result.index.index == NotFound.index or result.index.index == Unassigned.index) {
result.index.is_overflow = instance.backing_buf_used > max_index;
if (result.index.is_overflow) {
result.index.index = self.overflow_list.len();
if (reclaimed) |kv| {
result.index = kv.value;
} else {
result.index.index = instance.backing_buf_used;
instance.backing_buf_used += 1;
result.index.is_overflow = instance.backing_buf_used > max_index;
if (result.index.is_overflow) {
result.index.index = self.overflow_list.len();
} else {
result.index.index = instance.backing_buf_used;
instance.backing_buf_used += 1;
}
}
}

Expand All @@ -643,7 +676,15 @@ pub fn BSSMap(comptime ValueType: type, comptime count: anytype, comptime store_
}
}

/// Returns true if the entry was removed
/// Invalidate the cache entry for `key` so the next `getOrPut` returns
/// `.unknown`. The backing slot (and its value) is preserved in
/// `reclaimable` so the next `put()` for the same key overwrites it in
/// place instead of consuming a fresh slot. We do not `deinit()` the
/// value here because other cache entries may still hold raw pointers
/// into it (e.g. DirInfo.enclosing_package_json references a parent
/// directory's PackageJSON).
///
/// Returns true if the entry was removed.
pub fn remove(self: *Self, denormalized_key: []const u8) bool {
self.mutex.lock();
defer self.mutex.unlock();
Expand All @@ -654,28 +695,15 @@ pub fn BSSMap(comptime ValueType: type, comptime count: anytype, comptime store_
denormalized_key;

const _key = bun.hash(key);
return self.index.remove(_key);
// const index = self.index.get(_key) orelse return;
// switch (index) {
// Unassigned.index, NotFound.index => {
// self.index.remove(_key);
// },
// 0...max_index => {
// if (comptime hasDeinit(ValueType)) {
// instance.backing_buf[index].deinit();
// }

// instance.backing_buf[index] = undefined;
// },
// else => {
// const i = index - count;
// if (hasDeinit(ValueType)) {
// self.overflow_list.items[i].deinit();
// }
// self.overflow_list.items[index - count] = undefined;
// },
// }

const kv = self.index.fetchRemove(_key) orelse return false;
if (kv.value.index != NotFound.index and kv.value.index != Unassigned.index) {
// Best-effort: if we can't stash the hint, degrade to the
// pre-reclaim behavior (leak one slot) rather than crash
// mid–cache-bust. Unlike index.put in markNotFound above,
// this is a leak-reduction hint, not load-bearing state.
self.reclaimable.put(self.allocator, _key, kv.value) catch {};
}
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
return true;
}

pub fn values(self: *Self) []ValueType {
Expand Down
Loading
Loading