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" {