mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-07-12 12:19:47 +00:00
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.
This commit is contained in:
committed by
Mitchell Hashimoto
parent
7e14347c13
commit
65f953e8e8
@@ -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 {
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user