From 7e14347c130c3296136a28fb4255e94e1ca672e7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 11 Jul 2026 09:21:27 -0700 Subject: [PATCH 1/3] terminal: bound page map probe lengths #13292 Page maps previously allowed a 100% load factor. Once live entries and tombstones filled every slot, a missing-key lookup or insertion could scan the entire map. Reserve 20% of each offset hash map as insertion headroom and track that budget separately from the live count. Port the allocation-free in-place rehash from Zig HashMapUnmanaged so canonical insertion can rebuild fragmented probes and restore tombstone-exhausted headroom without growing the page. Assumed-capacity insertion now applies the same guard, preventing the headroom counter from wrapping. Pass the exact requested hyperlink count into map layout before load-factor scaling, avoiding a redundant power-of-two rounding step. Screen-level recovery now grows only when live hyperlinks actually fill usable capacity. ReleaseFast terminal benchmarks compare main with the combined map changes: | workload | before | after | speedup | |---|---:|---:|---:| | map churn | 1.253 s | 12.5 ms | 100x | | full OSC 8 stream | 3.576 s | 75.6 ms | 47x | | clear and redraw | 299.1 ms | 118.3 ms | 2.5x | Co-authored-by: Tim Culverhouse --- src/benchmark/HyperlinkMap.zig | 155 +++++++++++ src/benchmark/cli.zig | 2 + src/benchmark/main.zig | 1 + src/terminal/Screen.zig | 3 + src/terminal/hash_map.zig | 472 ++++++++++++++++++++++++++++++--- src/terminal/hyperlink.zig | 2 +- src/terminal/page.zig | 37 ++- 7 files changed, 631 insertions(+), 41 deletions(-) create mode 100644 src/benchmark/HyperlinkMap.zig diff --git a/src/benchmark/HyperlinkMap.zig b/src/benchmark/HyperlinkMap.zig new file mode 100644 index 000000000..166c3d4fb --- /dev/null +++ b/src/benchmark/HyperlinkMap.zig @@ -0,0 +1,155 @@ +//! Benchmark hyperlink cell-map lookups and remove/insert churn. +//! +//! Hyperlink cells are stored in a fixed-capacity, open-addressed hash map. +//! The `churn` mode models terminal output that repeatedly replaces cells in +//! a page whose hyperlink map is already close to full. This is particularly +//! useful for catching probe-length cliffs at high load factors. +const HyperlinkMap = @This(); + +const std = @import("std"); +const Allocator = std.mem.Allocator; +const terminal = @import("../terminal/main.zig"); +const hyperlink = @import("../terminal/hyperlink.zig"); +const Benchmark = @import("Benchmark.zig"); + +const log = std.log.scoped(.@"hyperlink-map-bench"); + +opts: Options, +page: terminal.Page, +link_id: hyperlink.Id, +entry_count: usize, + +pub const Options = struct { + /// Requested hyperlink working-set size. Must be a power of two and at + /// least 16. The map may reserve additional probe headroom. + entries: u16 = 4096, + + /// Percentage of the map populated before the timed operation. + /// Values above 100 are treated as 100. + @"load-percent": u8 = 100, + + /// Number of complete passes over the populated cells per step. + loops: u16 = 1, + + /// Operation to perform in the timed region. + mode: Mode = .churn, +}; + +pub const Mode = enum { + /// Look up every populated hyperlink cell. + lookup, + + /// Remove and reinsert every populated hyperlink cell. + churn, +}; + +pub fn create(alloc: Allocator, opts: Options) !*HyperlinkMap { + if (opts.entries < 16 or !std.math.isPowerOfTwo(opts.entries)) { + log.err("entries must be a power of two greater than or equal to 16", .{}); + return error.InvalidEntries; + } + + const ptr = try alloc.create(HyperlinkMap); + errdefer alloc.destroy(ptr); + + // The page requests one map slot per `hyperlink_cell_multiplier` set + // entries. Keep this relationship explicit so `entries` is the working + // set size under test regardless of the map's reserved probe headroom. + const set_entries = opts.entries / 16; + var page = try terminal.Page.init(.{ + .cols = opts.entries, + .rows = 1, + .hyperlink_bytes = @intCast( + @as(usize, set_entries) * @sizeOf(hyperlink.Set.Item), + ), + }); + errdefer page.deinit(); + + if (page.hyperlinkCapacity() < opts.entries) { + log.err("insufficient map capacity expected_at_least={} actual={}", .{ + opts.entries, + page.hyperlinkCapacity(), + }); + return error.UnexpectedCapacity; + } + + const link_id = try page.insertHyperlink(.{ + .id = .{ .implicit = 1 }, + .uri = "https://example.com/benchmark", + }); + + const load = @min(opts.@"load-percent", 100); + const entry_count = @max( + 1, + @divFloor(@as(usize, opts.entries) * load, 100), + ); + for (0..entry_count) |x| { + const rac = page.getRowAndCell(x, 0); + page.hyperlink_set.use(page.memory, link_id); + try page.setHyperlink(rac.row, rac.cell, link_id); + } + + ptr.* = .{ + .opts = opts, + .page = page, + .link_id = link_id, + .entry_count = entry_count, + }; + return ptr; +} + +pub fn destroy(self: *HyperlinkMap, alloc: Allocator) void { + self.page.deinit(); + alloc.destroy(self); +} + +pub fn benchmark(self: *HyperlinkMap) Benchmark { + return .init(self, .{ + .stepFn = switch (self.opts.mode) { + .lookup => stepLookup, + .churn => stepChurn, + }, + }); +} + +fn stepLookup(ptr: *anyopaque) Benchmark.Error!void { + const self: *HyperlinkMap = @ptrCast(@alignCast(ptr)); + + for (0..self.opts.loops) |_| { + for (0..self.entry_count) |x| { + const cell = self.page.getRowAndCell(x, 0).cell; + const id = self.page.lookupHyperlink(cell) orelse + return error.BenchmarkFailed; + std.mem.doNotOptimizeAway(id); + } + } +} + +fn stepChurn(ptr: *anyopaque) Benchmark.Error!void { + const self: *HyperlinkMap = @ptrCast(@alignCast(ptr)); + + for (0..self.opts.loops) |_| { + for (0..self.entry_count) |x| { + const rac = self.page.getRowAndCell(x, 0); + self.page.clearHyperlink(rac.cell); + self.page.hyperlink_set.use(self.page.memory, self.link_id); + self.page.setHyperlink(rac.row, rac.cell, self.link_id) catch + return error.BenchmarkFailed; + } + } +} + +test HyperlinkMap { + const alloc = std.testing.allocator; + + inline for (.{ Mode.lookup, Mode.churn }) |mode| { + const impl = try HyperlinkMap.create(alloc, .{ + .entries = 64, + .mode = mode, + }); + defer impl.destroy(alloc); + + const bench = impl.benchmark(); + _ = try bench.run(.once); + } +} diff --git a/src/benchmark/cli.zig b/src/benchmark/cli.zig index d51fd871e..dc4d7b8f4 100644 --- a/src/benchmark/cli.zig +++ b/src/benchmark/cli.zig @@ -9,6 +9,7 @@ pub const Action = enum { @"apc-parser", @"codepoint-width", @"grapheme-break", + @"hyperlink-map", @"page-compression", @"scrollback-compression", @"screen-clone", @@ -28,6 +29,7 @@ pub const Action = enum { pub fn Struct(comptime action: Action) type { return switch (action) { .@"apc-parser" => @import("ApcParser.zig"), + .@"hyperlink-map" => @import("HyperlinkMap.zig"), .@"screen-clone" => @import("ScreenClone.zig"), .@"page-compression" => @import("PageCompression.zig"), .@"scrollback-compression" => @import("ScrollbackCompression.zig"), diff --git a/src/benchmark/main.zig b/src/benchmark/main.zig index 61404c265..f22891c71 100644 --- a/src/benchmark/main.zig +++ b/src/benchmark/main.zig @@ -4,6 +4,7 @@ pub const CApi = @import("CApi.zig"); pub const TerminalStream = @import("TerminalStream.zig"); pub const CodepointWidth = @import("CodepointWidth.zig"); pub const GraphemeBreak = @import("GraphemeBreak.zig"); +pub const HyperlinkMap = @import("HyperlinkMap.zig"); pub const ScreenClone = @import("ScreenClone.zig"); pub const TerminalParser = @import("TerminalParser.zig"); pub const IsSymbol = @import("IsSymbol.zig"); diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 5ca5d957c..e02c0320b 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2567,6 +2567,9 @@ pub fn cursorSetHyperlink(self: *Screen) PageList.IncreaseCapacityError!void { page = new_node.page(); } + // Canonical map insertion rehashes tombstones in place. Reaching + // this error therefore means live entries fill the usable map + // capacity and the page must grow. _ = try self.increaseCapacity( self.cursor.page_pin.node, .hyperlink_bytes, diff --git a/src/terminal/hash_map.zig b/src/terminal/hash_map.zig index 5a38cc179..407e50290 100644 --- a/src/terminal/hash_map.zig +++ b/src/terminal/hash_map.zig @@ -42,12 +42,24 @@ const Offset = @import("size.zig").Offset; const OffsetBuf = @import("size.zig").OffsetBuf; const getOffset = @import("size.zig").getOffset; -pub fn AutoOffsetHashMap(comptime K: type, comptime V: type) type { - return OffsetHashMap(K, V, AutoContext(K)); +/// The default preserves the original behavior of allowing every raw slot to +/// be occupied. Callers can choose a lower value to bound probe lengths. +pub const default_max_load_percentage: u8 = 100; + +pub fn AutoOffsetHashMap( + comptime K: type, + comptime V: type, + comptime max_load_percentage: u8, +) type { + return OffsetHashMap(K, V, AutoContext(K), max_load_percentage); } -fn AutoHashMapUnmanaged(comptime K: type, comptime V: type) type { - return HashMapUnmanaged(K, V, AutoContext(K)); +fn AutoHashMapUnmanaged( + comptime K: type, + comptime V: type, + comptime max_load_percentage: u8, +) type { + return HashMapUnmanaged(K, V, AutoContext(K), max_load_percentage); } fn AutoContext(comptime K: type) type { @@ -64,12 +76,18 @@ pub fn OffsetHashMap( comptime K: type, comptime V: type, comptime Context: type, + comptime max_load_percentage: u8, ) type { return struct { const Self = @This(); /// This is the pointer-based map that we're wrapping. - pub const Unmanaged = HashMapUnmanaged(K, V, Context); + pub const Unmanaged = HashMapUnmanaged( + K, + V, + Context, + max_load_percentage, + ); pub const Layout = Unmanaged.Layout; /// This is the alignment that the base pointer must have. @@ -81,7 +99,7 @@ pub fn OffsetHashMap( /// HashMap with the given capacity. The base ptr must also be /// aligned to base_align. pub fn layout(cap: Unmanaged.Size) Layout { - return Unmanaged.layoutForCapacity(cap); + return Unmanaged.layoutForSize(cap); } /// Initialize a new HashMap with the given capacity and backing @@ -112,12 +130,15 @@ fn HashMapUnmanaged( comptime K: type, comptime V: type, comptime Context: type, + comptime max_load_percentage: u8, ) type { return struct { const Self = @This(); comptime { assert(@alignOf(Metadata) == 1); + assert(max_load_percentage > 0); + assert(max_load_percentage <= 100); } const header_align = @alignOf(Header); @@ -166,6 +187,11 @@ fn HashMapUnmanaged( keys: Offset(K), capacity: Size, size: Size, + + /// Number of insertions into free slots allowed before the map + /// must be rebuilt. Removing an entry creates a tombstone and + /// intentionally does not restore this count. + available: Size, }; /// Metadata for a slot. It can be in three states: empty, used or @@ -301,6 +327,7 @@ fn HashMapUnmanaged( const hdr = map.header(); hdr.capacity = layout.capacity; hdr.size = 0; + hdr.available = maxLoadForCapacity(layout.capacity); if (@sizeOf([*]K) != 0) hdr.keys = metadata_buf.member(K, layout.keys_start); if (@sizeOf([*]V) != 0) hdr.values = metadata_buf.member(V, layout.vals_start); map.initMetadatas(); @@ -322,6 +349,7 @@ fn HashMapUnmanaged( if (self.metadata) |_| { self.initMetadatas(); self.header().size = 0; + self.header().available = self.maxLoad(); } } @@ -347,6 +375,12 @@ fn HashMapUnmanaged( return self.header().capacity; } + /// Maximum number of occupied or tombstone slots before the map must + /// be rebuilt. This bounds unsuccessful probe lengths. + pub fn maxLoad(self: *const Self) Size { + return maxLoadForCapacity(self.capacity()); + } + pub fn iterator(self: *const Self) Iterator { return .{ .hm = self }; } @@ -430,6 +464,7 @@ fn HashMapUnmanaged( } const fingerprint = Metadata.takeFingerprint(hash); + if (metadata[0].isFree()) self.header().available -= 1; metadata[0].fill(fingerprint); self.keys()[idx] = key; self.values()[idx] = value; @@ -646,6 +681,8 @@ fn HashMapUnmanaged( return null; } + /// The get-or-put family may rehash a fragmented table. Any key or + /// value pointers previously returned by this map may be invalidated. pub fn getOrPut(self: *Self, key: K) Allocator.Error!GetOrPutResult { if (@sizeOf(Context) != 0) @compileError("Cannot infer context " ++ @typeName(Context) ++ ", call getOrPutContext instead."); @@ -665,6 +702,18 @@ fn HashMapUnmanaged( } pub fn getOrPutContextAdapted(self: *Self, key: anytype, key_ctx: anytype) Allocator.Error!GetOrPutResult { self.growIfNeeded(1) catch |err| { + // Canonical lookups can rebuild resident keys in place. If + // live entries still fit, insertion headroom was consumed by + // tombstones rather than live load. + if (comptime @TypeOf(key) == K and + @TypeOf(key_ctx) == Context) + { + if (self.header().size < self.maxLoad()) { + self.rehash(key_ctx); + return self.getOrPutAssumeCapacityAdapted(key, key_ctx); + } + } + // If allocation fails, try to do the lookup anyway. // If we find an existing item, we can return it. // Otherwise return the error, we could not add another. @@ -705,6 +754,7 @@ fn HashMapUnmanaged( var idx = @as(usize, @truncate(hash & mask)); var first_tombstone_idx: usize = self.capacity(); // invalid index + var tombstones: Size = 0; var metadata = self.metadata.? + idx; while (!metadata[0].isFree() and limit != 0) { if (metadata[0].isUsed() and metadata[0].fingerprint == fingerprint) { @@ -724,8 +774,24 @@ fn HashMapUnmanaged( .found_existing = true, }; } - } else if (first_tombstone_idx == self.capacity() and metadata[0].isTombstone()) { - first_tombstone_idx = idx; + } else if (metadata[0].isTombstone()) { + if (first_tombstone_idx == self.capacity()) { + first_tombstone_idx = idx; + } + + // Rehash once this probe demonstrates meaningful + // fragmentation. Only canonical lookups have the + // context required to rehash resident K values. + if (comptime @TypeOf(key) == K and @TypeOf(ctx) == Context) { + tombstones += 1; + // Amortize the O(capacity) rebuild and avoid doing it + // for an otherwise healthy, nearly full table. + const threshold = @max(self.capacity() / 8, 1); + if (tombstones >= threshold) { + self.rehash(ctx); + return self.getOrPutAssumeCapacityAdapted(key, ctx); + } + } } limit -= 1; @@ -739,6 +805,25 @@ fn HashMapUnmanaged( metadata = self.metadata.? + idx; } + if (metadata[0].isFree()) { + // Assume-capacity callers can arrive here after a removal + // consumed all insertion headroom. Canonical lookups can + // rebuild resident keys before consuming another free slot. + if (self.header().available == 0) { + if (comptime @TypeOf(key) == K and + @TypeOf(ctx) == Context) + { + assert(self.header().size < self.maxLoad()); + self.rehash(ctx); + return self.getOrPutAssumeCapacityAdapted(key, ctx); + } + + // Adapted contexts cannot hash resident keys to rehash. + // Their caller must honor the assume-capacity contract. + assert(self.header().available > 0); + } + self.header().available -= 1; + } metadata[0].fill(fingerprint); const new_key = &self.keys()[idx]; const new_value = &self.values()[idx]; @@ -827,9 +912,94 @@ fn HashMapUnmanaged( @memset(@as([*]u8, @ptrCast(self.metadata.?))[0 .. @sizeOf(Metadata) * self.capacity()], 0); } + /// Rebuild the map in place, removing all tombstones. This moves + /// entries and invalidates existing key and value pointers. + pub fn rehash(self: *Self, ctx: Context) void { + const mask = self.capacity() - 1; + + const metadata = self.metadata.?; + const keys_ptr = self.keys(); + const values_ptr = self.values(); + var curr: Size = 0; + + // Mark used buckets as awaiting rehash and clear tombstones. + while (curr < self.capacity()) : (curr += 1) { + metadata[curr].fingerprint = Metadata.free; + } + + curr = 0; + while (curr < self.capacity()) { + if (!metadata[curr].isUsed()) { + assert(metadata[curr].isFree()); + curr += 1; + continue; + } + + const hash = ctx.hash(keys_ptr[curr]); + const fingerprint = Metadata.takeFingerprint(hash); + var idx = @as(usize, @truncate(hash & mask)); + + // For each bucket, rehash to an index: + // 1) before the cursor, probed into a free slot, or + // 2) equal to the cursor, no need to move, or + // 3) ahead of the cursor, probing over already rehashed. + while ((idx < curr and metadata[idx].isUsed()) or + (idx > curr and metadata[idx].fingerprint == Metadata.tombstone)) + { + idx = (idx + 1) & mask; + } + + if (idx < curr) { + assert(metadata[idx].isFree()); + metadata[idx].fill(fingerprint); + keys_ptr[idx] = keys_ptr[curr]; + values_ptr[idx] = values_ptr[curr]; + + metadata[curr].used = 0; + assert(metadata[curr].isFree()); + keys_ptr[curr] = undefined; + values_ptr[curr] = undefined; + + curr += 1; + } else if (idx == curr) { + metadata[idx].fingerprint = fingerprint; + curr += 1; + } else { + assert(metadata[idx].fingerprint != Metadata.tombstone); + metadata[idx].fingerprint = Metadata.tombstone; + if (metadata[idx].isUsed()) { + mem.swap(K, &keys_ptr[curr], &keys_ptr[idx]); + mem.swap(V, &values_ptr[curr], &values_ptr[idx]); + } else { + metadata[idx].used = 1; + keys_ptr[idx] = keys_ptr[curr]; + values_ptr[idx] = values_ptr[curr]; + + metadata[curr].fingerprint = Metadata.free; + metadata[curr].used = 0; + keys_ptr[curr] = undefined; + values_ptr[curr] = undefined; + + curr += 1; + } + } + } + + // Rehashing removes every tombstone, so all unused load-factor + // headroom is available for insertions again. + self.header().available = self.maxLoad() - self.header().size; + } + fn growIfNeeded(self: *Self, new_count: Size) Allocator.Error!void { - const available = self.capacity() - self.header().size; - if (new_count > available) return error.OutOfMemory; + if (new_count > self.header().available) return error.OutOfMemory; + } + + fn maxLoadForCapacity(cap: Size) Size { + if (cap == 0) return 0; + return @intCast(@divFloor( + @as(u64, cap) * max_load_percentage, + 100, + )); } /// The memory layout for the underlying buffer for a given capacity. @@ -888,6 +1058,37 @@ fn HashMapUnmanaged( .capacity = new_capacity, }; } + + /// Returns a layout with enough raw slots to hold `new_size` entries + /// at the configured maximum load factor. + pub fn layoutForSize(new_size: Size) Layout { + if (new_size == 0) return layoutForCapacity(0); + + // Scale the requested number of entries up to the raw slot count + // required by the load factor. Widen first so `new_size * 100` + // cannot overflow Size. + const minimum_capacity = std.math.divCeil( + u64, + @as(u64, new_size) * 100, + max_load_percentage, + ) catch unreachable; + + // Capacities must be powers of two, so the largest capacity that + // fits in Size is the highest bit rather than maxInt(Size). + const max_capacity = @as(u64, 1) << + (@typeInfo(Size).int.bits - 1); + if (minimum_capacity > max_capacity) { + return layoutForCapacity(@intCast(max_capacity)); + } + + // Linear probing uses a mask for wraparound, which requires the + // final raw capacity to be rounded up to a power of two. + const raw_capacity = std.math.ceilPowerOfTwo( + u64, + minimum_capacity, + ) catch unreachable; + return layoutForCapacity(@intCast(raw_capacity)); + } }; } @@ -896,7 +1097,7 @@ const expect = std.testing.expect; const expectEqual = std.testing.expectEqual; test "HashMap basic usage" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const alloc = testing.allocator; const cap = 16; @@ -931,7 +1132,7 @@ test "HashMap basic usage" { } test "HashMap ensureTotalCapacity" { - const Map = AutoHashMapUnmanaged(i32, i32); + const Map = AutoHashMapUnmanaged(i32, i32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -951,7 +1152,7 @@ test "HashMap ensureTotalCapacity" { } test "HashMap ensureUnusedCapacity with tombstones" { - const Map = AutoHashMapUnmanaged(i32, i32); + const Map = AutoHashMapUnmanaged(i32, i32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -969,7 +1170,7 @@ test "HashMap ensureUnusedCapacity with tombstones" { } test "HashMap clearRetainingCapacity" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = 16; const alloc = testing.allocator; @@ -1000,7 +1201,7 @@ test "HashMap clearRetainingCapacity" { } test "HashMap ensureTotalCapacity with existing elements" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = Map.minimal_capacity; const alloc = testing.allocator; @@ -1019,7 +1220,7 @@ test "HashMap ensureTotalCapacity with existing elements" { } test "HashMap remove" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -1057,7 +1258,7 @@ test "HashMap remove" { } test "HashMap reverse removes" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -1085,7 +1286,7 @@ test "HashMap reverse removes" { } test "HashMap multiple removes on same metadata" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -1128,7 +1329,7 @@ test "HashMap multiple removes on same metadata" { } test "HashMap put and remove loop in random order" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = 64; const alloc = testing.allocator; @@ -1166,7 +1367,7 @@ test "HashMap put and remove loop in random order" { } test "HashMap put" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -1197,7 +1398,7 @@ test "HashMap put" { } test "HashMap put full load" { - const Map = AutoHashMapUnmanaged(usize, usize); + const Map = AutoHashMapUnmanaged(usize, usize, default_max_load_percentage); const cap = 16; const alloc = testing.allocator; @@ -1213,7 +1414,7 @@ test "HashMap put full load" { } test "HashMap putAssumeCapacity" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -1248,7 +1449,7 @@ test "HashMap putAssumeCapacity" { } test "HashMap repeat putAssumeCapacity/remove" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -1283,8 +1484,175 @@ test "HashMap repeat putAssumeCapacity/remove" { try expectEqual(map.count(), limit); } +test "HashMap clobber insert rehashes exhausted headroom" { + const Context = struct { + pub fn hash(_: @This(), key: u32) u64 { + return key; + } + + pub fn eql(_: @This(), a: u32, b: u32) bool { + return a == b; + } + }; + const Map = HashMapUnmanaged(u32, u32, Context, 80); + const cap = 16; + + const alloc = testing.allocator; + const layout = Map.layoutForCapacity(cap); + const buf = try alloc.alignedAlloc(u8, Map.base_align, layout.total_size); + defer alloc.free(buf); + var map = Map.init(.init(buf), layout); + + const max_load = map.maxLoad(); + for (0..max_load) |i| { + map.putAssumeCapacityNoClobberContext( + @intCast(i), + @intCast(i), + .{}, + ); + } + + try expect(map.removeContext(0, .{})); + map.putAssumeCapacityContext(15, 15, .{}); + + try expectEqual(max_load, map.count()); + try expectEqual(15, map.getContext(15, .{}).?); + for (map.metadata.?[0..map.capacity()]) |metadata| { + try expect(!metadata.isTombstone()); + } +} + +test "HashMap getOrPut rehashes a fragmented probe" { + const Context = struct { + pub fn hash(_: @This(), _: u32) u64 { + return 0; + } + + pub fn eql(_: @This(), a: u32, b: u32) bool { + return a == b; + } + }; + const AdaptedContext = struct { + pub fn hash(_: @This(), _: []const u8) u64 { + return 0; + } + + pub fn eql(_: @This(), adapted: []const u8, key: u32) bool { + return std.fmt.parseInt(u32, adapted, 10) catch unreachable == key; + } + }; + const Map = HashMapUnmanaged( + u32, + u32, + Context, + default_max_load_percentage, + ); + const cap = 32; + + const alloc = testing.allocator; + const layout = Map.layoutForCapacity(cap); + const buf = try alloc.alignedAlloc(u8, Map.base_align, layout.total_size); + defer alloc.free(buf); + var map = Map.init(.init(buf), layout); + + for (0..cap) |i| { + map.putAssumeCapacityNoClobberContext(@intCast(i), @intCast(i), .{}); + } + + // Rehashing preserves a table at the supported 100% live occupancy. + map.rehash(.{}); + try expectEqual(cap, map.count()); + for (0..cap) |i| { + try expectEqual(i, map.getContext(@intCast(i), .{}).?); + } + + for (0..cap / 2) |i| { + try expect(map.removeContext(@intCast(i), .{})); + } + + var tombstones: usize = 0; + for (map.metadata.?[0..map.capacity()]) |metadata| { + if (metadata.isTombstone()) tombstones += 1; + } + try expectEqual(cap / 2, tombstones); + + // An adapted lookup cannot rehash without the context for resident keys. + const adapted = try map.getOrPutAdapted("31", AdaptedContext{}); + try expect(adapted.found_existing); + try expectEqual(cap - 1, adapted.value_ptr.*); + tombstones = 0; + for (map.metadata.?[0..map.capacity()]) |metadata| { + if (metadata.isTombstone()) tombstones += 1; + } + try expectEqual(cap / 2, tombstones); + + // Looking up an existing key beyond the tombstones rehashes and retries + // before returning pointers into the map. + const gop = try map.getOrPutContext(cap - 1, .{}); + try expect(gop.found_existing); + try expectEqual(cap - 1, gop.value_ptr.*); + try expectEqual(cap / 2, map.count()); + + tombstones = 0; + for (map.metadata.?[0..map.capacity()]) |metadata| { + if (metadata.isTombstone()) tombstones += 1; + } + try expectEqual(0, tombstones); + + for (cap / 2..cap) |i| { + try expectEqual(i, map.getContext(@intCast(i), .{}).?); + } +} + +test "HashMap rehash with real hashes" { + const Map = AutoHashMapUnmanaged( + u32, + u32, + default_max_load_percentage, + ); + const cap = 512; + + const alloc = testing.allocator; + const layout = Map.layoutForCapacity(cap); + const buf = try alloc.alignedAlloc(u8, Map.base_align, layout.total_size); + defer alloc.free(buf); + var map = Map.init(.init(buf), layout); + + for (0..cap) |i| { + map.putAssumeCapacityNoClobber(@intCast(i), @intCast(i)); + } + + map.rehash(undefined); + try expectEqual(cap, map.count()); + for (0..cap) |i| { + try expectEqual(i, map.get(@intCast(i)).?); + } + + var expected_count: usize = cap; + for (0..cap) |i| { + if (i % 3 == 0) { + try expect(map.remove(@intCast(i))); + expected_count -= 1; + } + } + + map.rehash(undefined); + try expectEqual(expected_count, map.count()); + for (0..cap) |i| { + if (i % 3 == 0) { + try expectEqual(null, map.get(@intCast(i))); + } else { + try expectEqual(i, map.get(@intCast(i)).?); + } + } + + for (map.metadata.?[0..map.capacity()]) |metadata| { + try expect(!metadata.isTombstone()); + } +} + test "HashMap getOrPut" { - const Map = AutoHashMapUnmanaged(u32, u32); + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -1313,7 +1681,7 @@ test "HashMap getOrPut" { } test "HashMap basic hash map usage" { - const Map = AutoHashMapUnmanaged(i32, i32); + const Map = AutoHashMapUnmanaged(i32, i32, default_max_load_percentage); const cap = 32; const alloc = testing.allocator; @@ -1364,7 +1732,7 @@ test "HashMap basic hash map usage" { } test "HashMap ensureUnusedCapacity" { - const Map = AutoHashMapUnmanaged(u64, u64); + const Map = AutoHashMapUnmanaged(u64, u64, default_max_load_percentage); const cap = 64; const alloc = testing.allocator; @@ -1378,7 +1746,7 @@ test "HashMap ensureUnusedCapacity" { } test "HashMap removeByPtr" { - const Map = AutoHashMapUnmanaged(i32, u64); + const Map = AutoHashMapUnmanaged(i32, u64, default_max_load_percentage); const cap = 64; const alloc = testing.allocator; @@ -1409,7 +1777,7 @@ test "HashMap removeByPtr" { } test "HashMap removeByPtr 0 sized key" { - const Map = AutoHashMapUnmanaged(i32, u64); + const Map = AutoHashMapUnmanaged(i32, u64, default_max_load_percentage); const cap = 64; const alloc = testing.allocator; @@ -1433,7 +1801,7 @@ test "HashMap removeByPtr 0 sized key" { } test "HashMap repeat fetchRemove" { - const Map = AutoHashMapUnmanaged(u64, void); + const Map = AutoHashMapUnmanaged(u64, void, default_max_load_percentage); const cap = 64; const alloc = testing.allocator; @@ -1461,7 +1829,11 @@ test "HashMap repeat fetchRemove" { } test "OffsetHashMap basic usage" { - const OffsetMap = AutoOffsetHashMap(u32, u32); + const OffsetMap = AutoOffsetHashMap( + u32, + u32, + default_max_load_percentage, + ); const cap = 16; const alloc = testing.allocator; @@ -1496,7 +1868,11 @@ test "OffsetHashMap basic usage" { } test "OffsetHashMap remake map" { - const OffsetMap = AutoOffsetHashMap(u32, u32); + const OffsetMap = AutoOffsetHashMap( + u32, + u32, + default_max_load_percentage, + ); const cap = 16; const alloc = testing.allocator; @@ -1516,12 +1892,44 @@ test "OffsetHashMap remake map" { } } +test "OffsetHashMap maximum load leaves probe headroom" { + const OffsetMap = AutoOffsetHashMap(u32, u32, 80); + const alloc = testing.allocator; + const requested_size = 16; + const layout = OffsetMap.layout(requested_size); + const buf = try alloc.alignedAlloc( + u8, + OffsetMap.base_align, + layout.total_size, + ); + defer alloc.free(buf); + + const offset_map = OffsetMap.init(.init(buf), layout); + var map = offset_map.map(buf); + + try testing.expect(map.capacity() > requested_size); + try testing.expect(map.maxLoad() >= requested_size); + try testing.expect(map.maxLoad() < map.capacity()); + + for (0..requested_size) |i| try map.put(@intCast(i), @intCast(i)); + for (0..100) |_| { + for (0..requested_size) |i| { + try testing.expect(map.remove(@intCast(i))); + try map.put(@intCast(i), @intCast(i)); + } + } + + for (0..requested_size) |i| { + try testing.expectEqual(@as(u32, @intCast(i)), map.get(@intCast(i))); + } +} + test "layoutForCapacity no overflow for large capacity" { // Test that layoutForCapacity correctly handles large capacities without overflow. // Prior to the fix, new_capacity (u32) was multiplied before widening to usize, // causing overflow when new_capacity * @sizeOf(K) exceeded 2^32. // See: https://github.com/ghostty-org/ghostty/issues/9862 - const Map = AutoHashMapUnmanaged(u64, u64); + const Map = AutoHashMapUnmanaged(u64, u64, default_max_load_percentage); // Use 2^30 capacity - this would overflow in u32 when multiplied by @sizeOf(u64)=8 // 0x40000000 * 8 = 0x2_0000_0000 which wraps to 0 in u32 diff --git a/src/terminal/hyperlink.zig b/src/terminal/hyperlink.zig index 94f86466c..35a16a2ae 100644 --- a/src/terminal/hyperlink.zig +++ b/src/terminal/hyperlink.zig @@ -20,7 +20,7 @@ pub const Id = size.HyperlinkCountInt; // The mapping of cell to hyperlink. We use an offset hash map to save space // since its very unlikely a cell is a hyperlink, so its a waste to store // the hyperlink ID in the cell itself. -pub const Map = AutoOffsetHashMap(Offset(Cell), Id); +pub const Map = AutoOffsetHashMap(Offset(Cell), Id, 80); /// A fully decoded hyperlink that may or may not have its /// memory within a page. The memory location of this is dependent diff --git a/src/terminal/page.zig b/src/terminal/page.zig index c2759d2a2..ca6e2e220 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -92,7 +92,11 @@ const grapheme_chunk = grapheme_chunk_len * @sizeOf(u21); const GraphemeAlloc = BitmapAllocator(grapheme_chunk); const grapheme_count_default = GraphemeAlloc.bitmap_bit_size; pub const grapheme_bytes_default = grapheme_count_default * grapheme_chunk; -const GraphemeMap = AutoOffsetHashMap(Offset(Cell), Offset(u21).Slice); +const GraphemeMap = AutoOffsetHashMap( + Offset(Cell), + Offset(u21).Slice, + hash_map.default_max_load_percentage, +); /// The allocator used for shared utf8-encoded strings within a page. /// Note the chunk size below is the minimum size of a single allocation @@ -770,11 +774,11 @@ pub const Page = struct { } } - // The hyperlink_map capacity in layout() is computed as: - // hyperlink_count * hyperlink_cell_multiplier (rounded to power of 2) - // We need enough hyperlink_bytes so that when layout() computes - // the map capacity, it can accommodate all hyperlink cells. This - // is unit tested. + // layout() requests `hyperlink_count * hyperlink_cell_multiplier` + // usable map entries. The map layout adds load-factor headroom and + // rounds the raw slot count to a power of two. We need enough + // hyperlink_bytes for that requested entry count to accommodate all + // hyperlink cells. This is unit tested. const hyperlink_cap = cap: { const hyperlink_count = id_set.count(); const hyperlink_set_cap = hyperlink.Set.capacityForCount(hyperlink_count); @@ -1494,7 +1498,7 @@ pub const Page = struct { /// Returns the hyperlink capacity for the page. This isn't the byte /// size but the number of unique cells that can have hyperlink data. pub inline fn hyperlinkCapacity(self: *const Page) usize { - return self.hyperlink_map.map(self.memory).capacity(); + return self.hyperlink_map.map(self.memory).maxLoad(); } /// Set the graphemes for the given cell. This asserts that the cell @@ -1765,7 +1769,7 @@ pub const Page = struct { u32, hyperlink_count * hyperlink_cell_multiplier, ) orelse break :count std.math.maxInt(u32); - break :count std.math.ceilPowerOfTwoAssert(u32, mult); + break :count mult; }; const hyperlink_map_layout = hyperlink.Map.layout(hyperlink_map_count); const hyperlink_map_start = alignForward(usize, hyperlink_set_end, hyperlink.Map.base_align.toByteUnits()); @@ -4211,3 +4215,20 @@ test "Page exactRowCapacity hyperlink map capacity for many cells" { try testing.expect(cloned_cell.hyperlink); } } + +test "Page layout avoids double rounding hyperlink map capacity" { + const hyperlink_count = 3; + const layout = Page.layout(.{ + .cols = 1, + .rows = 1, + .hyperlink_bytes = hyperlink_count * @sizeOf(hyperlink.Set.Item), + }); + + // Three set entries request 48 usable map entries. Scaling that for the + // 80% load factor needs 60 raw slots, which rounds once to 64. Rounding + // the request before applying the load factor would allocate 128 slots. + try std.testing.expectEqual( + @as(u32, 64), + layout.hyperlink_map_layout.capacity, + ); +} From 65f953e8e82f7dfd9e5eb4e04538ad88a71f2452 Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Sat, 11 Jul 2026 09:21:38 -0700 Subject: [PATCH 2/3] terminal: avoid duplicate probes when moving cell data Hyperlink and grapheme maps are keyed by cell offset. Moving their data removes the source entry and reinserts it at a destination known to be absent. Generic clobbering insertion still searches the probe sequence to rule out a duplicate. Use no-clobber insertion for these moves so the first available tombstone can be reused. If a destination reaches a free slot after tombstones exhaust insertion headroom, rebuild the map in place and retry before decrementing the budget. This keeps the known-absent fast path safe at every load state. The ReleaseFast movement benchmark improves from 133.7 ms to 126.4 ms, a 5.5% reduction. The linked-line control remains neutral at 395.4 ms before and 395.7 ms after. --- src/terminal/hash_map.zig | 65 +++++++++++++++++++++++++++++++++++++-- src/terminal/page.zig | 4 +-- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/terminal/hash_map.zig b/src/terminal/hash_map.zig index 407e50290..b14b9478b 100644 --- a/src/terminal/hash_map.zig +++ b/src/terminal/hash_map.zig @@ -425,7 +425,12 @@ fn HashMapUnmanaged( } pub fn putNoClobberContext(self: *Self, key: K, value: V, ctx: Context) Allocator.Error!void { assert(!self.containsContext(key, ctx)); - try self.growIfNeeded(1); + self.growIfNeeded(1) catch |err| { + // Live entries still fit, so an assume-capacity insertion can + // either recycle a tombstone or rehash if its probe reaches a + // genuinely free slot. + if (self.header().size >= self.maxLoad()) return err; + }; self.putAssumeCapacityNoClobberContext(key, value, ctx); } @@ -464,7 +469,22 @@ fn HashMapUnmanaged( } const fingerprint = Metadata.takeFingerprint(hash); - if (metadata[0].isFree()) self.header().available -= 1; + if (metadata[0].isFree()) { + // Removal does not restore insertion headroom. A move can + // therefore remove its source, probe to a different free + // slot, and arrive here with no headroom left. Rehash before + // consuming that slot so the counter and metadata agree. + if (self.header().available == 0) { + assert(self.header().size < self.maxLoad()); + self.rehash(ctx); + return self.putAssumeCapacityNoClobberContext( + key, + value, + ctx, + ); + } + self.header().available -= 1; + } metadata[0].fill(fingerprint); self.keys()[idx] = key; self.values()[idx] = value; @@ -1484,6 +1504,47 @@ test "HashMap repeat putAssumeCapacity/remove" { try expectEqual(map.count(), limit); } +test "HashMap no-clobber move rehashes exhausted headroom" { + const Context = struct { + pub fn hash(_: @This(), key: u32) u64 { + return key; + } + + pub fn eql(_: @This(), a: u32, b: u32) bool { + return a == b; + } + }; + const Map = HashMapUnmanaged(u32, u32, Context, 80); + const cap = 16; + + const alloc = testing.allocator; + const layout = Map.layoutForCapacity(cap); + const buf = try alloc.alignedAlloc(u8, Map.base_align, layout.total_size); + defer alloc.free(buf); + var map = Map.init(.init(buf), layout); + + // Fill all insertion headroom with keys in the first part of the table. + const max_load = map.maxLoad(); + for (0..max_load) |i| { + map.putAssumeCapacityNoClobberContext( + @intCast(i), + @intCast(i), + .{}, + ); + } + + // Model a managed-cell move: removing the source leaves a tombstone but + // does not restore headroom, and the destination hashes to a free slot. + try expect(map.removeContext(0, .{})); + map.putAssumeCapacityNoClobberContext(15, 15, .{}); + + try expectEqual(max_load, map.count()); + try expectEqual(15, map.getContext(15, .{}).?); + for (map.metadata.?[0..map.capacity()]) |metadata| { + try expect(!metadata.isTombstone()); + } +} + test "HashMap clobber insert rehashes exhausted headroom" { const Context = struct { pub fn hash(_: @This(), key: u32) u64 { diff --git a/src/terminal/page.zig b/src/terminal/page.zig index ca6e2e220..ca6d9854c 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -1481,7 +1481,7 @@ pub const Page = struct { const entry = map.getEntry(src_offset).?; const value = entry.value_ptr.*; map.removeByPtr(entry.key_ptr); - map.putAssumeCapacity(dst_offset, value); + map.putAssumeCapacityNoClobber(dst_offset, value); // NOTE: We must not set src/dst.hyperlink here because this // function is used in various cases where we swap cell contents @@ -1629,7 +1629,7 @@ pub const Page = struct { const entry = map.getEntry(src_offset).?; const value = entry.value_ptr.*; map.removeByPtr(entry.key_ptr); - map.putAssumeCapacity(dst_offset, value); + map.putAssumeCapacityNoClobber(dst_offset, value); } /// Clear the graphemes for a given cell. From fedd42e8d66b2ba5665031000ce842ca0535a610 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 11 Jul 2026 14:19:32 -0700 Subject: [PATCH 3/3] terminal: use backward-shift deletion in page maps Page maps handled removal with tombstones, which a fixed-capacity map can never outgrow. Keeping probe lengths bounded required an insertion headroom counter, an allocation-free rehash, and five separate recovery paths with subtle invariants: removal created a tombstone without restoring headroom, so the counter could reach zero while the map was half empty and every insertion path had to be prepared to rebuild. Replace tombstones with backward-shift deletion (Knuth vol. 3, 6.4 algorithm R). Removal restores the table to the state it would be in had the key never been inserted, so probe chains stay canonical at all times and fragmentation cannot accumulate by construction. This deletes the headroom counter, the in-place rehash, and every recovery path. Insertion no longer invalidates pointers; removal may now move other entries instead, and no caller holds entry pointers across removals. Removal costs a cluster scan instead of a byte write, paying incrementally what tombstones deferred to later probes and periodic rebuilds. Free slots remain all-zero bytes so probe loops keep fusing the state and fingerprint checks into single-byte compares. A randomized oracle test against the stdlib map and a canonical-placement invariant check cover the new removal path, including full tables where no free slot terminates the shift. ReleaseFast benchmarks against the prior map commits: | workload | delta | |---|---:| | map churn, 50% load | 1.23x faster | | map churn, 75% load | 1.15x faster | | map churn, max load | 1.07x slower | | map lookup | neutral | | clear and redraw stream | 1.32x faster | | full OSC 8 stream | 1.06x faster | | linked-line movement | neutral | --- src/terminal/Screen.zig | 6 +- src/terminal/hash_map.zig | 628 +++++++++++++++----------------------- 2 files changed, 249 insertions(+), 385 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index e02c0320b..6c4db3237 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2567,9 +2567,9 @@ pub fn cursorSetHyperlink(self: *Screen) PageList.IncreaseCapacityError!void { page = new_node.page(); } - // Canonical map insertion rehashes tombstones in place. Reaching - // this error therefore means live entries fill the usable map - // capacity and the page must grow. + // The hyperlink map is fixed-capacity, so reaching this error + // means live entries fill the usable map capacity and the page + // must grow. _ = try self.increaseCapacity( self.cursor.page_pin.node, .hyperlink_bytes, diff --git a/src/terminal/hash_map.zig b/src/terminal/hash_map.zig index b14b9478b..d8feddcd1 100644 --- a/src/terminal/hash_map.zig +++ b/src/terminal/hash_map.zig @@ -23,27 +23,33 @@ //! because our terminal page representation is backed by a single large //! allocation so we can give the HashMap a slice of memory to operate in. //! -//! I haven't carefully benchmarked this implementation against other hash -//! map implementations. It's possible using some of the newer variants out -//! there would be better. However, I trust the built-in version is pretty good -//! and its more important to get the terminal page representation working -//! first then we can measure and improve this later if we find it to be a -//! bottleneck. +//! This fork diverges from the stdlib in one significant way: removal uses +//! backward-shift deletion (Knuth vol. 3, section 6.4, algorithm R) rather +//! than tombstones. A fixed-capacity map cannot outgrow tombstone buildup +//! the way an allocating map does, so tombstones require either unbounded +//! probe lengths or periodic in-place rebuilds with subtle bookkeeping. +//! Backward-shift deletion instead restores the table after every removal +//! to the exact state it would be in had the removed key never been +//! inserted. Probe chains are therefore always minimal for the insertion +//! order, there is no fragmentation to repair, and lookup cost depends only +//! on the current load factor. +//! +//! Pointer stability: insertion never moves existing entries, but removal +//! may move *other* entries within a probe cluster. Any key or value +//! pointers previously returned by the map must be considered invalidated +//! by any removal. const std = @import("std"); const assert = @import("../quirks.zig").inlineAssert; -const autoHash = std.hash.autoHash; -const math = std.math; const mem = std.mem; const Allocator = mem.Allocator; -const Wyhash = std.hash.Wyhash; const Offset = @import("size.zig").Offset; const OffsetBuf = @import("size.zig").OffsetBuf; const getOffset = @import("size.zig").getOffset; -/// The default preserves the original behavior of allowing every raw slot to -/// be occupied. Callers can choose a lower value to bound probe lengths. +/// The default allows every raw slot to be occupied. Callers whose maps see +/// removal-heavy churn should choose a lower value to bound probe lengths. pub const default_max_load_percentage: u8 = 100; pub fn AutoOffsetHashMap( @@ -123,9 +129,9 @@ pub fn OffsetHashMap( } /// Fork of stdlib.HashMap as of Zig 0.12 modified to use offsets for -/// the key/values pointer. The metadata is still a pointer to limit -/// the amount of arithmetic required to access it. See the file comment -/// for full details. +/// the key/values pointer, and backward-shift deletion in place of +/// tombstones. The metadata is still a pointer to limit the amount of +/// arithmetic required to access it. See the file comment for full details. fn HashMapUnmanaged( comptime K: type, comptime V: type, @@ -160,10 +166,6 @@ fn HashMapUnmanaged( /// Pointer to the metadata. metadata: ?[*]Metadata = null, - // This is purely empirical and not a /very smart magic constant™/. - /// Capacity of the first grow when bootstrapping the hashmap. - const minimal_capacity = 8; - // This hashmap is specially designed for sizes that fit in a u32. pub const Size = u32; @@ -187,18 +189,11 @@ fn HashMapUnmanaged( keys: Offset(K), capacity: Size, size: Size, - - /// Number of insertions into free slots allowed before the map - /// must be rebuilt. Removing an entry creates a tombstone and - /// intentionally does not restore this count. - available: Size, }; - /// Metadata for a slot. It can be in three states: empty, used or - /// tombstone. Tombstones indicate that an entry was previously used, - /// they are a simple way to handle removal. - /// To this state, we add 7 bits from the slot's key hash. These are - /// used as a fast way to disambiguate between entries without + /// Metadata for a slot. It can be in two states: free or used. + /// To the used state, we add 7 bits from the slot's key hash. These + /// are used as a fast way to disambiguate between entries without /// having to use the equality function. If two fingerprints are /// different, we know that we don't have to compare the keys at all. /// The 7 bits are the highest ones from a 64 bit hash. This way, not @@ -208,28 +203,23 @@ fn HashMapUnmanaged( /// Not using the equality function means we don't have to read into /// the entries array, likely avoiding a cache miss and a potentially /// costly function call. - const Metadata = packed struct { + const Metadata = packed struct(u8) { const FingerPrint = u7; - const free: FingerPrint = 0; - const tombstone: FingerPrint = 1; - - fingerprint: FingerPrint = free, + fingerprint: FingerPrint = 0, used: u1 = 0, - const slot_free = @as(u8, @bitCast(Metadata{ .fingerprint = free })); - const slot_tombstone = @as(u8, @bitCast(Metadata{ .fingerprint = tombstone })); - pub fn isUsed(self: Metadata) bool { return self.used == 1; } - pub fn isTombstone(self: Metadata) bool { - return @as(u8, @bitCast(self)) == slot_tombstone; - } - pub fn isFree(self: Metadata) bool { - return @as(u8, @bitCast(self)) == slot_free; + // A free slot is always the all-zero byte: `fill` sets the + // used bit and removal zeroes the whole byte. Comparing the + // full byte (rather than testing the used bit) lets the + // optimizer fuse this with the fingerprint comparison in + // probe loops into single-byte compares. + return @as(u8, @bitCast(self)) == 0; } pub fn takeFingerprint(hash: Hash) FingerPrint { @@ -242,11 +232,6 @@ fn HashMapUnmanaged( self.used = 1; self.fingerprint = fp; } - - pub fn remove(self: *Metadata) void { - self.used = 0; - self.fingerprint = tombstone; - } }; comptime { @@ -254,6 +239,9 @@ fn HashMapUnmanaged( assert(@alignOf(Metadata) == 1); } + /// Iterates the entries of the map. Any mutation of the map + /// invalidates the iterator: removal may move entries across the + /// iteration cursor. pub const Iterator = struct { hm: *const Self, index: Size = 0, @@ -327,7 +315,6 @@ fn HashMapUnmanaged( const hdr = map.header(); hdr.capacity = layout.capacity; hdr.size = 0; - hdr.available = maxLoadForCapacity(layout.capacity); if (@sizeOf([*]K) != 0) hdr.keys = metadata_buf.member(K, layout.keys_start); if (@sizeOf([*]V) != 0) hdr.values = metadata_buf.member(V, layout.vals_start); map.initMetadatas(); @@ -337,7 +324,7 @@ fn HashMapUnmanaged( pub fn ensureTotalCapacity(self: *Self, new_size: Size) Allocator.Error!void { if (new_size > self.header().size) { - try self.growIfNeeded(new_size - self.header().size); + try self.checkCapacity(new_size - self.header().size); } } @@ -349,7 +336,6 @@ fn HashMapUnmanaged( if (self.metadata) |_| { self.initMetadatas(); self.header().size = 0; - self.header().available = self.maxLoad(); } } @@ -375,8 +361,9 @@ fn HashMapUnmanaged( return self.header().capacity; } - /// Maximum number of occupied or tombstone slots before the map must - /// be rebuilt. This bounds unsuccessful probe lengths. + /// Maximum number of entries the map will hold. This is less than + /// capacity when max_load_percentage is below 100, which keeps free + /// slots in every probe chain and bounds probe lengths. pub fn maxLoad(self: *const Self) Size { return maxLoadForCapacity(self.capacity()); } @@ -425,12 +412,7 @@ fn HashMapUnmanaged( } pub fn putNoClobberContext(self: *Self, key: K, value: V, ctx: Context) Allocator.Error!void { assert(!self.containsContext(key, ctx)); - self.growIfNeeded(1) catch |err| { - // Live entries still fit, so an assume-capacity insertion can - // either recycle a tombstone or rehash if its probe reaches a - // genuinely free slot. - if (self.header().size >= self.maxLoad()) return err; - }; + try self.checkCapacity(1); self.putAssumeCapacityNoClobberContext(key, value, ctx); } @@ -458,6 +440,9 @@ fn HashMapUnmanaged( pub fn putAssumeCapacityNoClobberContext(self: *Self, key: K, value: V, ctx: Context) void { assert(!self.containsContext(key, ctx)); + // A free slot must exist for the probe below to terminate. + assert(self.header().size < self.capacity()); + const hash = ctx.hash(key); const mask = self.capacity() - 1; var idx = @as(usize, @truncate(hash & mask)); @@ -468,24 +453,7 @@ fn HashMapUnmanaged( metadata = self.metadata.? + idx; } - const fingerprint = Metadata.takeFingerprint(hash); - if (metadata[0].isFree()) { - // Removal does not restore insertion headroom. A move can - // therefore remove its source, probe to a different free - // slot, and arrive here with no headroom left. Rehash before - // consuming that slot so the counter and metadata agree. - if (self.header().available == 0) { - assert(self.header().size < self.maxLoad()); - self.rehash(ctx); - return self.putAssumeCapacityNoClobberContext( - key, - value, - ctx, - ); - } - self.header().available -= 1; - } - metadata[0].fill(fingerprint); + metadata[0].fill(Metadata.takeFingerprint(hash)); self.keys()[idx] = key; self.values()[idx] = value; self.header().size += 1; @@ -531,31 +499,22 @@ fn HashMapUnmanaged( } /// If there is an `Entry` with a matching key, it is deleted from - /// the hash map, and then returned from this function. + /// the hash map, and then returned from this function. Removal may + /// move other entries: any previously returned key or value + /// pointers are invalidated. pub fn fetchRemove(self: *Self, key: K) ?KV { if (@sizeOf(Context) != 0) @compileError("Cannot infer context " ++ @typeName(Context) ++ ", call fetchRemoveContext instead."); return self.fetchRemoveContext(key, undefined); } pub fn fetchRemoveContext(self: *Self, key: K, ctx: Context) ?KV { - return self.fetchRemoveAdapted(key, ctx); - } - pub fn fetchRemoveAdapted(self: *Self, key: anytype, ctx: anytype) ?KV { - if (self.getIndex(key, ctx)) |idx| { - const old_key = &self.keys()[idx]; - const old_val = &self.values()[idx]; - const result = KV{ - .key = old_key.*, - .value = old_val.*, - }; - self.metadata.?[idx].remove(); - old_key.* = undefined; - old_val.* = undefined; - self.header().size -= 1; - return result; - } - - return null; + const idx = self.getIndex(key, ctx) orelse return null; + const result = KV{ + .key = self.keys()[idx], + .value = self.values()[idx], + }; + self.removeByIndexContext(idx, ctx); + return result; } /// Find the index containing the data for the given key. @@ -579,7 +538,7 @@ fn HashMapUnmanaged( } const mask = self.capacity() - 1; const fingerprint = Metadata.takeFingerprint(hash); - // Don't loop indefinitely when there are no empty slots. + // Don't loop indefinitely when there are no free slots. var limit = self.capacity(); var idx = @as(usize, @truncate(hash & mask)); @@ -701,8 +660,6 @@ fn HashMapUnmanaged( return null; } - /// The get-or-put family may rehash a fragmented table. Any key or - /// value pointers previously returned by this map may be invalidated. pub fn getOrPut(self: *Self, key: K) Allocator.Error!GetOrPutResult { if (@sizeOf(Context) != 0) @compileError("Cannot infer context " ++ @typeName(Context) ++ ", call getOrPutContext instead."); @@ -721,22 +678,10 @@ fn HashMapUnmanaged( return self.getOrPutContextAdapted(key, key_ctx); } pub fn getOrPutContextAdapted(self: *Self, key: anytype, key_ctx: anytype) Allocator.Error!GetOrPutResult { - self.growIfNeeded(1) catch |err| { - // Canonical lookups can rebuild resident keys in place. If - // live entries still fit, insertion headroom was consumed by - // tombstones rather than live load. - if (comptime @TypeOf(key) == K and - @TypeOf(key_ctx) == Context) - { - if (self.header().size < self.maxLoad()) { - self.rehash(key_ctx); - return self.getOrPutAssumeCapacityAdapted(key, key_ctx); - } - } - - // If allocation fails, try to do the lookup anyway. - // If we find an existing item, we can return it. - // Otherwise return the error, we could not add another. + self.checkCapacity(1) catch |err| { + // The map is full. Try to do the lookup anyway; if we find + // an existing item, we can return it. Otherwise return the + // error, we could not add another. const index = self.getIndex(key, key_ctx) orelse return err; return GetOrPutResult{ .key_ptr = &self.keys()[index], @@ -773,8 +718,6 @@ fn HashMapUnmanaged( var limit = self.capacity(); var idx = @as(usize, @truncate(hash & mask)); - var first_tombstone_idx: usize = self.capacity(); // invalid index - var tombstones: Size = 0; var metadata = self.metadata.? + idx; while (!metadata[0].isFree() and limit != 0) { if (metadata[0].isUsed() and metadata[0].fingerprint == fingerprint) { @@ -794,24 +737,6 @@ fn HashMapUnmanaged( .found_existing = true, }; } - } else if (metadata[0].isTombstone()) { - if (first_tombstone_idx == self.capacity()) { - first_tombstone_idx = idx; - } - - // Rehash once this probe demonstrates meaningful - // fragmentation. Only canonical lookups have the - // context required to rehash resident K values. - if (comptime @TypeOf(key) == K and @TypeOf(ctx) == Context) { - tombstones += 1; - // Amortize the O(capacity) rebuild and avoid doing it - // for an otherwise healthy, nearly full table. - const threshold = @max(self.capacity() / 8, 1); - if (tombstones >= threshold) { - self.rehash(ctx); - return self.getOrPutAssumeCapacityAdapted(key, ctx); - } - } } limit -= 1; @@ -819,31 +744,12 @@ fn HashMapUnmanaged( metadata = self.metadata.? + idx; } - if (first_tombstone_idx < self.capacity()) { - // Cheap try to lower probing lengths after deletions. Recycle a tombstone. - idx = first_tombstone_idx; - metadata = self.metadata.? + idx; - } + // The caller guaranteed capacity for at least one new entry, so + // the probe must have ended at a free slot. Anything else means + // the assume-capacity contract was violated and we would be + // silently overwriting a live entry. + assert(metadata[0].isFree()); - if (metadata[0].isFree()) { - // Assume-capacity callers can arrive here after a removal - // consumed all insertion headroom. Canonical lookups can - // rebuild resident keys before consuming another free slot. - if (self.header().available == 0) { - if (comptime @TypeOf(key) == K and - @TypeOf(ctx) == Context) - { - assert(self.header().size < self.maxLoad()); - self.rehash(ctx); - return self.getOrPutAssumeCapacityAdapted(key, ctx); - } - - // Adapted contexts cannot hash resident keys to rehash. - // Their caller must honor the assume-capacity contract. - assert(self.header().available > 0); - } - self.header().available -= 1; - } metadata[0].fill(fingerprint); const new_key = &self.keys()[idx]; const new_value = &self.values()[idx]; @@ -885,37 +791,74 @@ fn HashMapUnmanaged( return self.getIndex(key, ctx) != null; } - fn removeByIndex(self: *Self, idx: usize) void { - self.metadata.?[idx].remove(); - self.keys()[idx] = undefined; - self.values()[idx] = undefined; + /// Remove the entry at the given index using backward-shift deletion + /// (Knuth vol. 3, section 6.4, algorithm R): rather than marking the + /// slot with a tombstone, restore the table to the state it would be + /// in had the removed key never been inserted. Any entry whose probe + /// sequence passes over the hole is moved into it, which moves the + /// hole further along the cluster, until the cluster ends at a free + /// slot. + fn removeByIndexContext(self: *Self, idx: usize, ctx: Context) void { + const mask: usize = self.capacity() - 1; + const metadata = self.metadata.?; + const keys_ptr = self.keys(); + const values_ptr = self.values(); + + // A completely full table has no free slot to terminate the + // scan, so bound it to one full cycle. That is sufficient: the + // hole only ever moves forward to slots the scan has already + // visited, so each entry needs to be considered exactly once. + var hole = idx; + var j = idx; + var limit = self.capacity() - 1; + while (limit != 0) : (limit -= 1) { + j = (j + 1) & mask; + if (metadata[j].isFree()) break; + + // The entry at `j` may move into the hole only if the hole + // lies on its probe path, i.e. cyclically within [home, j). + // Otherwise the move would place it before its home slot + // and lookups could no longer find it. + const home: usize = @truncate(ctx.hash(keys_ptr[j]) & mask); + if (((hole -% home) & mask) < ((j -% home) & mask)) { + metadata[hole] = metadata[j]; + keys_ptr[hole] = keys_ptr[j]; + values_ptr[hole] = values_ptr[j]; + hole = j; + } + } + + metadata[hole] = .{}; + keys_ptr[hole] = undefined; + values_ptr[hole] = undefined; self.header().size -= 1; } /// If there is an `Entry` with a matching key, it is deleted from /// the hash map, and this function returns true. Otherwise this - /// function returns false. + /// function returns false. Removal may move other entries: any + /// previously returned key or value pointers are invalidated. pub fn remove(self: *Self, key: K) bool { if (@sizeOf(Context) != 0) @compileError("Cannot infer context " ++ @typeName(Context) ++ ", call removeContext instead."); return self.removeContext(key, undefined); } pub fn removeContext(self: *Self, key: K, ctx: Context) bool { - return self.removeAdapted(key, ctx); - } - pub fn removeAdapted(self: *Self, key: anytype, ctx: anytype) bool { - if (self.getIndex(key, ctx)) |idx| { - self.removeByIndex(idx); - return true; - } - - return false; + const idx = self.getIndex(key, ctx) orelse return false; + self.removeByIndexContext(idx, ctx); + return true; } /// Delete the entry with key pointed to by key_ptr from the hash map. /// key_ptr is assumed to be a valid pointer to a key that is present - /// in the hash map. + /// in the hash map. Removal may move other entries: any previously + /// returned key or value pointers are invalidated. pub fn removeByPtr(self: *Self, key_ptr: *K) void { + if (@sizeOf(Context) != 0) + @compileError("Cannot infer context " ++ @typeName(Context) ++ ", call removeByPtrContext instead."); + return self.removeByPtrContext(key_ptr, undefined); + } + pub fn removeByPtrContext(self: *Self, key_ptr: *K, ctx: Context) void { // TODO: replace with pointer subtraction once supported by zig // if @sizeOf(K) == 0 then there is at most one item in the hash // map, which is assumed to exist as key_ptr must be valid. This @@ -925,93 +868,19 @@ fn HashMapUnmanaged( else 0; - self.removeByIndex(idx); + self.removeByIndexContext(idx, ctx); } fn initMetadatas(self: *Self) void { @memset(@as([*]u8, @ptrCast(self.metadata.?))[0 .. @sizeOf(Metadata) * self.capacity()], 0); } - /// Rebuild the map in place, removing all tombstones. This moves - /// entries and invalidates existing key and value pointers. - pub fn rehash(self: *Self, ctx: Context) void { - const mask = self.capacity() - 1; - - const metadata = self.metadata.?; - const keys_ptr = self.keys(); - const values_ptr = self.values(); - var curr: Size = 0; - - // Mark used buckets as awaiting rehash and clear tombstones. - while (curr < self.capacity()) : (curr += 1) { - metadata[curr].fingerprint = Metadata.free; - } - - curr = 0; - while (curr < self.capacity()) { - if (!metadata[curr].isUsed()) { - assert(metadata[curr].isFree()); - curr += 1; - continue; - } - - const hash = ctx.hash(keys_ptr[curr]); - const fingerprint = Metadata.takeFingerprint(hash); - var idx = @as(usize, @truncate(hash & mask)); - - // For each bucket, rehash to an index: - // 1) before the cursor, probed into a free slot, or - // 2) equal to the cursor, no need to move, or - // 3) ahead of the cursor, probing over already rehashed. - while ((idx < curr and metadata[idx].isUsed()) or - (idx > curr and metadata[idx].fingerprint == Metadata.tombstone)) - { - idx = (idx + 1) & mask; - } - - if (idx < curr) { - assert(metadata[idx].isFree()); - metadata[idx].fill(fingerprint); - keys_ptr[idx] = keys_ptr[curr]; - values_ptr[idx] = values_ptr[curr]; - - metadata[curr].used = 0; - assert(metadata[curr].isFree()); - keys_ptr[curr] = undefined; - values_ptr[curr] = undefined; - - curr += 1; - } else if (idx == curr) { - metadata[idx].fingerprint = fingerprint; - curr += 1; - } else { - assert(metadata[idx].fingerprint != Metadata.tombstone); - metadata[idx].fingerprint = Metadata.tombstone; - if (metadata[idx].isUsed()) { - mem.swap(K, &keys_ptr[curr], &keys_ptr[idx]); - mem.swap(V, &values_ptr[curr], &values_ptr[idx]); - } else { - metadata[idx].used = 1; - keys_ptr[idx] = keys_ptr[curr]; - values_ptr[idx] = values_ptr[curr]; - - metadata[curr].fingerprint = Metadata.free; - metadata[curr].used = 0; - keys_ptr[curr] = undefined; - values_ptr[curr] = undefined; - - curr += 1; - } - } - } - - // Rehashing removes every tombstone, so all unused load-factor - // headroom is available for insertions again. - self.header().available = self.maxLoad() - self.header().size; - } - - fn growIfNeeded(self: *Self, new_count: Size) Allocator.Error!void { - if (new_count > self.header().available) return error.OutOfMemory; + /// Returns an error if the map cannot hold `new_count` more entries. + /// This map is fixed-capacity so nothing can be done to make room; + /// the caller must grow the backing memory and rebuild the map. + fn checkCapacity(self: *Self, new_count: Size) Allocator.Error!void { + const available = self.maxLoad() - self.header().size; + if (new_count > available) return error.OutOfMemory; } fn maxLoadForCapacity(cap: Size) Size { @@ -1116,6 +985,26 @@ const testing = std.testing; const expect = std.testing.expect; const expectEqual = std.testing.expectEqual; +/// Verify the canonical placement invariant that backward-shift deletion +/// maintains: every used entry is reachable from its home slot without +/// crossing a free slot. This is exactly the property lookups depend on. +fn expectCanonical(map: anytype, ctx: anytype) !void { + const cap = map.capacity(); + const mask = cap - 1; + var used: usize = 0; + for (0..cap) |idx| { + const metadata = map.metadata.?[idx]; + if (!metadata.isUsed()) continue; + used += 1; + + var probe: usize = @truncate(ctx.hash(map.keys()[idx]) & mask); + while (probe != idx) : (probe = (probe + 1) & mask) { + try expect(map.metadata.?[probe].isUsed()); + } + } + try expectEqual(map.count(), used); +} + test "HashMap basic usage" { const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); @@ -1171,7 +1060,7 @@ test "HashMap ensureTotalCapacity" { try testing.expect(initial_capacity == map.capacity()); } -test "HashMap ensureUnusedCapacity with tombstones" { +test "HashMap ensureUnusedCapacity with removals" { const Map = AutoHashMapUnmanaged(i32, i32, default_max_load_percentage); const cap = 32; @@ -1222,7 +1111,7 @@ test "HashMap clearRetainingCapacity" { test "HashMap ensureTotalCapacity with existing elements" { const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); - const cap = Map.minimal_capacity; + const cap = 8; const alloc = testing.allocator; const layout = Map.layoutForCapacity(cap); @@ -1232,11 +1121,11 @@ test "HashMap ensureTotalCapacity with existing elements" { try map.put(0, 0); try expectEqual(map.count(), 1); - try expectEqual(map.capacity(), Map.minimal_capacity); + try expectEqual(map.capacity(), cap); try testing.expectError(error.OutOfMemory, map.ensureTotalCapacity(65)); try expectEqual(map.count(), 1); - try expectEqual(map.capacity(), Map.minimal_capacity); + try expectEqual(map.capacity(), cap); } test "HashMap remove" { @@ -1504,7 +1393,7 @@ test "HashMap repeat putAssumeCapacity/remove" { try expectEqual(map.count(), limit); } -test "HashMap no-clobber move rehashes exhausted headroom" { +test "HashMap no-clobber move after remove at max load" { const Context = struct { pub fn hash(_: @This(), key: u32) u64 { return key; @@ -1523,7 +1412,7 @@ test "HashMap no-clobber move rehashes exhausted headroom" { defer alloc.free(buf); var map = Map.init(.init(buf), layout); - // Fill all insertion headroom with keys in the first part of the table. + // Fill the map to its maximum load. const max_load = map.maxLoad(); for (0..max_load) |i| { map.putAssumeCapacityNoClobberContext( @@ -1533,82 +1422,41 @@ test "HashMap no-clobber move rehashes exhausted headroom" { ); } - // Model a managed-cell move: removing the source leaves a tombstone but - // does not restore headroom, and the destination hashes to a free slot. - try expect(map.removeContext(0, .{})); - map.putAssumeCapacityNoClobberContext(15, 15, .{}); + // Model a managed-cell move: remove the source and insert the value at + // a destination known to be absent. This must work at maximum load for + // any number of moves since removal genuinely frees a slot. + for (0..100) |i| { + const src: u32 = @intCast(i); + const dst: u32 = @intCast(i + max_load); + try expect(map.removeContext(src, .{})); + map.putAssumeCapacityNoClobberContext(dst, dst, .{}); - try expectEqual(max_load, map.count()); - try expectEqual(15, map.getContext(15, .{}).?); - for (map.metadata.?[0..map.capacity()]) |metadata| { - try expect(!metadata.isTombstone()); + try expectEqual(max_load, map.count()); + try expectEqual(dst, map.getContext(dst, .{}).?); + try expectCanonical(&map, Context{}); } } -test "HashMap clobber insert rehashes exhausted headroom" { - const Context = struct { - pub fn hash(_: @This(), key: u32) u64 { - return key; - } - - pub fn eql(_: @This(), a: u32, b: u32) bool { - return a == b; - } - }; - const Map = HashMapUnmanaged(u32, u32, Context, 80); - const cap = 16; - - const alloc = testing.allocator; - const layout = Map.layoutForCapacity(cap); - const buf = try alloc.alignedAlloc(u8, Map.base_align, layout.total_size); - defer alloc.free(buf); - var map = Map.init(.init(buf), layout); - - const max_load = map.maxLoad(); - for (0..max_load) |i| { - map.putAssumeCapacityNoClobberContext( - @intCast(i), - @intCast(i), - .{}, - ); - } - - try expect(map.removeContext(0, .{})); - map.putAssumeCapacityContext(15, 15, .{}); - - try expectEqual(max_load, map.count()); - try expectEqual(15, map.getContext(15, .{}).?); - for (map.metadata.?[0..map.capacity()]) |metadata| { - try expect(!metadata.isTombstone()); - } -} - -test "HashMap getOrPut rehashes a fragmented probe" { +test "HashMap removal keeps colliding clusters findable" { + // All keys hash to the same home slot near the end of the table so + // that clusters wrap around the index mask. This exercises the cyclic + // arithmetic in backward-shift deletion. const Context = struct { pub fn hash(_: @This(), _: u32) u64 { - return 0; + return 14; } pub fn eql(_: @This(), a: u32, b: u32) bool { return a == b; } }; - const AdaptedContext = struct { - pub fn hash(_: @This(), _: []const u8) u64 { - return 0; - } - - pub fn eql(_: @This(), adapted: []const u8, key: u32) bool { - return std.fmt.parseInt(u32, adapted, 10) catch unreachable == key; - } - }; const Map = HashMapUnmanaged( u32, u32, Context, default_max_load_percentage, ); - const cap = 32; + const cap = 16; const alloc = testing.allocator; const layout = Map.layoutForCapacity(cap); @@ -1616,62 +1464,33 @@ test "HashMap getOrPut rehashes a fragmented probe" { defer alloc.free(buf); var map = Map.init(.init(buf), layout); - for (0..cap) |i| { + // Fill half the table: the cluster spans the wraparound point. + for (0..cap / 2) |i| { map.putAssumeCapacityNoClobberContext(@intCast(i), @intCast(i), .{}); } - // Rehashing preserves a table at the supported 100% live occupancy. - map.rehash(.{}); - try expectEqual(cap, map.count()); - for (0..cap) |i| { - try expectEqual(i, map.getContext(@intCast(i), .{}).?); - } + // Remove from the middle of the cluster and verify all remaining + // entries stay findable after every removal. + var removed: usize = 0; + for ([_]u32{ 3, 0, 7, 4, 1, 6, 2, 5 }) |key| { + try expect(map.removeContext(key, .{})); + removed += 1; - for (0..cap / 2) |i| { - try expect(map.removeContext(@intCast(i), .{})); - } - - var tombstones: usize = 0; - for (map.metadata.?[0..map.capacity()]) |metadata| { - if (metadata.isTombstone()) tombstones += 1; - } - try expectEqual(cap / 2, tombstones); - - // An adapted lookup cannot rehash without the context for resident keys. - const adapted = try map.getOrPutAdapted("31", AdaptedContext{}); - try expect(adapted.found_existing); - try expectEqual(cap - 1, adapted.value_ptr.*); - tombstones = 0; - for (map.metadata.?[0..map.capacity()]) |metadata| { - if (metadata.isTombstone()) tombstones += 1; - } - try expectEqual(cap / 2, tombstones); - - // Looking up an existing key beyond the tombstones rehashes and retries - // before returning pointers into the map. - const gop = try map.getOrPutContext(cap - 1, .{}); - try expect(gop.found_existing); - try expectEqual(cap - 1, gop.value_ptr.*); - try expectEqual(cap / 2, map.count()); - - tombstones = 0; - for (map.metadata.?[0..map.capacity()]) |metadata| { - if (metadata.isTombstone()) tombstones += 1; - } - try expectEqual(0, tombstones); - - for (cap / 2..cap) |i| { - try expectEqual(i, map.getContext(@intCast(i), .{}).?); + for (0..cap / 2) |i| { + const k: u32 = @intCast(i); + const v = map.getContext(k, .{}); + if (map.containsContext(k, .{})) { + try expectEqual(k, v.?); + } + } + try expectEqual(cap / 2 - removed, map.count()); + try expectCanonical(&map, Context{}); } } -test "HashMap rehash with real hashes" { - const Map = AutoHashMapUnmanaged( - u32, - u32, - default_max_load_percentage, - ); - const cap = 512; +test "HashMap removal from a completely full table" { + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); + const cap = 64; const alloc = testing.allocator; const layout = Map.layoutForCapacity(cap); @@ -1679,37 +1498,82 @@ test "HashMap rehash with real hashes" { defer alloc.free(buf); var map = Map.init(.init(buf), layout); + // A 100% load factor allows filling every raw slot, so removal cannot + // rely on a free slot to terminate its cluster scan. for (0..cap) |i| { map.putAssumeCapacityNoClobber(@intCast(i), @intCast(i)); } - - map.rehash(undefined); try expectEqual(cap, map.count()); + + // Remove every other key, verifying everything else stays findable. + var expected: usize = cap; for (0..cap) |i| { - try expectEqual(i, map.get(@intCast(i)).?); + if (i % 2 != 0) continue; + try expect(map.remove(@intCast(i))); + expected -= 1; + try expectEqual(expected, map.count()); } - var expected_count: usize = cap; for (0..cap) |i| { - if (i % 3 == 0) { - try expect(map.remove(@intCast(i))); - expected_count -= 1; - } - } - - map.rehash(undefined); - try expectEqual(expected_count, map.count()); - for (0..cap) |i| { - if (i % 3 == 0) { + if (i % 2 == 0) { try expectEqual(null, map.get(@intCast(i))); } else { try expectEqual(i, map.get(@intCast(i)).?); } } + try expectCanonical(&map, AutoContext(u32){}); +} - for (map.metadata.?[0..map.capacity()]) |metadata| { - try expect(!metadata.isTombstone()); +test "HashMap random operations against an oracle" { + const Map = AutoHashMapUnmanaged(u32, u32, default_max_load_percentage); + const cap = 64; + + const alloc = testing.allocator; + const layout = Map.layoutForCapacity(cap); + const buf = try alloc.alignedAlloc(u8, Map.base_align, layout.total_size); + defer alloc.free(buf); + var map = Map.init(.init(buf), layout); + + var oracle: std.AutoHashMapUnmanaged(u32, u32) = .empty; + defer oracle.deinit(alloc); + + var prng = std.Random.DefaultPrng.init(0xdeadbeef); + const random = prng.random(); + + // A small key space forces frequent hits, misses, and re-insertions + // at every load factor from empty to completely full. + const key_space = cap + cap / 2; + for (0..20_000) |_| { + const key = random.uintLessThan(u32, key_space); + switch (random.uintLessThan(u8, 4)) { + 0, 1 => { + const value = random.int(u32); + if (map.put(key, value)) { + try oracle.put(alloc, key, value); + } else |_| { + // Map is full: the oracle must not know this key + // (put on an existing key always succeeds). + try expect(!oracle.contains(key)); + try expectEqual(map.count(), map.capacity()); + } + }, + 2 => try expectEqual( + oracle.remove(key), + map.remove(key), + ), + 3 => try expectEqual(oracle.get(key), map.get(key)), + else => unreachable, + } + + try expectEqual(oracle.count(), map.count()); } + + // Final full comparison plus the canonical placement invariant. + var it = oracle.iterator(); + while (it.next()) |entry| { + try expectEqual(entry.value_ptr.*, map.get(entry.key_ptr.*).?); + } + try expectCanonical(&map, AutoContext(u32){}); } test "HashMap getOrPut" {