From 641df6cd2ab19441e08681757eb02b496a98d991 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 11 Jul 2026 16:43:19 -0700 Subject: [PATCH] terminal: reuse resident explicit hyperlinks Starting an OSC 8 hyperlink copied its ID and URI into page memory before the ref-counted set could determine that the same live entry already existed. Repeated explicit IDs therefore paid allocation, copy, and free costs for data that was immediately discarded. Add adapted lookup support to RefCountedSet and let decoded hyperlinks hash and compare directly against page-resident entries. Probe explicit IDs before allocating strings and increment the existing reference on a match. Implicit IDs remain on the original path because they are generated uniquely and an early probe would always miss. An explicit and implicit parity test pins decoded and resident hashes to the same bit pattern so the adapted lookup cannot silently miss after either implementation changes. This also lets a live link be reused when the page string allocator has no free chunks. The old path requested a transient string copy first and could force an unnecessary page-capacity increase before de-duplicating. ReleaseFast medians use seven sequential runs after two warmups and report user CPU time. The stream reasserts one live explicit link 25,000 times while overwriting the same cell. | workload | before | after | change | |---|---:|---:|---:| | repeated explicit link | 437.925 ms | 438.051 ms | +0.0% | The isolated throughput effect is neutral; this change removes transient allocation and capacity growth rather than improving the steady-state parser path. --- src/terminal/hyperlink.zig | 88 +++++++++++++++++++++++++++++--- src/terminal/page.zig | 54 +++++++++++++++++++- src/terminal/ref_counted_set.zig | 12 ++++- 3 files changed, 142 insertions(+), 12 deletions(-) diff --git a/src/terminal/hyperlink.zig b/src/terminal/hyperlink.zig index 2a8310ec1..1e61dc21c 100644 --- a/src/terminal/hyperlink.zig +++ b/src/terminal/hyperlink.zig @@ -76,6 +76,45 @@ pub const Hyperlink = struct { return .{ .id = id, .uri = uri }; } + + /// Hash a decoded hyperlink identically to its page-resident form so it + /// can be used for an adapted lookup before allocating page strings. + /// This must remain bit-identical to PageEntry.hash. + pub fn hash(self: *const Hyperlink) u64 { + var hasher = Wyhash.init(0); + autoHash(&hasher, std.meta.activeTag(self.id)); + switch (self.id) { + .implicit => |v| autoHash(&hasher, v), + .explicit => |v| autoHashStrat(&hasher, v, .Deep), + } + autoHashStrat(&hasher, self.uri, .Deep); + return hasher.final(); + } + + /// Compare decoded input against a page-resident entry without copying + /// the input strings into the page first. + pub fn eqlPageEntry( + self: *const Hyperlink, + other: *const PageEntry, + other_base: anytype, + ) bool { + switch (self.id) { + .implicit => |id| switch (other.id) { + .implicit => |other_id| if (id != other_id) return false, + .explicit => return false, + }, + .explicit => |id| switch (other.id) { + .implicit => return false, + .explicit => |other_id| if (!std.mem.eql( + u8, + id, + other_id.slice(other_base), + )) return false, + }, + } + + return std.mem.eql(u8, self.uri, other.uri.slice(other_base)); + } }; /// A hyperlink that has been committed to page memory. This @@ -145,6 +184,8 @@ pub const PageEntry = struct { return copy; } + /// This must remain bit-identical to Hyperlink.hash so decoded links can + /// probe the resident set before allocating page strings. pub fn hash(self: *const PageEntry, base: anytype) u64 { var hasher = Wyhash.init(0); autoHash(&hasher, std.meta.activeTag(self.id)); @@ -226,16 +267,24 @@ pub const Set = RefCountedSet( /// if different from the destination page. src_page: ?*const Page = null, - pub fn hash(self: *const @This(), link: PageEntry) u64 { - return link.hash((self.src_page orelse self.page.?).memory); + pub fn hash(self: *const @This(), link: anytype) u64 { + return switch (@TypeOf(link)) { + Hyperlink => link.hash(), + PageEntry => link.hash((self.src_page orelse self.page.?).memory), + else => @compileError("unsupported hyperlink lookup type"), + }; } - pub fn eql(self: *const @This(), a: PageEntry, b: PageEntry) bool { - return a.eql( - (self.src_page orelse self.page.?).memory, - &b, - self.page.?.memory, - ); + pub fn eql(self: *const @This(), a: anytype, b: PageEntry) bool { + return switch (@TypeOf(a)) { + Hyperlink => a.eqlPageEntry(&b, self.page.?.memory), + PageEntry => a.eql( + (self.src_page orelse self.page.?).memory, + &b, + self.page.?.memory, + ), + else => @compileError("unsupported hyperlink lookup type"), + }; } pub fn deleted(self: *const @This(), link: PageEntry) void { @@ -243,3 +292,26 @@ pub const Set = RefCountedSet( } }, ); + +test "decoded and resident hyperlink hashes match" { + var page = try Page.init(.{ + .cols = 1, + .rows = 1, + }); + defer page.deinit(); + + for ([_]Hyperlink{ + .{ + .id = .{ .explicit = "explicit-id" }, + .uri = "https://example.com/explicit", + }, + .{ + .id = .{ .implicit = 42 }, + .uri = "https://example.com/implicit", + }, + }) |link| { + const id = try page.insertHyperlink(link); + const entry = page.hyperlink_set.get(page.memory, id); + try std.testing.expectEqual(link.hash(), entry.hash(page.memory)); + } +} diff --git a/src/terminal/page.zig b/src/terminal/page.zig index d68a576b0..636a348fe 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -1337,8 +1337,9 @@ pub const Page = struct { /// Convert a hyperlink into a page entry, returning the ID. /// - /// This does not de-dupe any strings, so if the URI, explicit ID, - /// etc. is already in the strings table this will duplicate it. + /// A matching live hyperlink entry is reused before allocating. On a + /// miss, this does not independently de-duplicate strings, so strings + /// shared with some different hyperlink entry are copied. /// /// To release the memory associated with the given hyperlink, /// release the ID from the `hyperlink_set`. If the refcount reaches @@ -1348,6 +1349,23 @@ pub const Page = struct { self: *Page, link: hyperlink.Hyperlink, ) InsertHyperlinkError!hyperlink.Id { + // Probe explicit IDs with the decoded input before allocating page + // strings. OSC 8 producers commonly re-assert them for many spans; + // if the link is still used by a cell, its strings can be shared. + // Implicit IDs are generated uniquely, so probing them is guaranteed + // to miss in normal operation and would only duplicate add's lookup. + switch (link.id) { + .implicit => {}, + .explicit => if (self.hyperlink_set.lookupAdapted( + self.memory, + link, + hyperlink.Set.Context{ .page = self }, + )) |id| { + self.hyperlink_set.use(self.memory, id); + return id; + }, + } + // Insert our URI into the page strings table. const page_uri: Offset(u8).Slice = uri: { const buf = self.string_alloc.alloc( @@ -3054,6 +3072,38 @@ test "Page cloneFrom hyperlinks exact capacity" { try testing.expectEqual(page2.hyperlinkCount(), page.hyperlinkCount()); } +test "Page insertHyperlink reuses live resident strings" { + var page = try Page.init(.{ + .cols = 10, + .rows = 10, + .hyperlink_bytes = 8 * @sizeOf(hyperlink.Set.Item), + .string_bytes = 256, + }); + defer page.deinit(); + + const link: hyperlink.Hyperlink = .{ + .id = .{ .explicit = "same-id" }, + .uri = "https://example.com/same-link", + }; + const first = try page.insertHyperlink(link); + + // Consume the remaining string storage. Re-inserting a live entry must + // not require space for a transient copy of strings that it will discard. + while (true) { + _ = page.string_alloc.alloc(u8, page.memory, 1) catch break; + } + const used = page.string_alloc.usedBytes(page.memory); + const second = try page.insertHyperlink(link); + + try testing.expectEqual(first, second); + try testing.expectEqual(used, page.string_alloc.usedBytes(page.memory)); + try testing.expectEqual(@as(usize, 1), page.hyperlink_set.count()); + try testing.expectEqual(@as(size.CellCountInt, 2), page.hyperlink_set.refCount( + page.memory, + first, + )); +} + test "Page cloneFrom graphemes" { var page = try Page.init(.{ .cols = 10, diff --git a/src/terminal/ref_counted_set.zig b/src/terminal/ref_counted_set.zig index f978c268e..cefd6a2d9 100644 --- a/src/terminal/ref_counted_set.zig +++ b/src/terminal/ref_counted_set.zig @@ -37,9 +37,11 @@ const OffsetBuf = size.OffsetBuf; /// `Context` /// A type containing methods to define behaviors. /// -/// - `fn hash(*Context, T) u64` - Return a hash for an item. +/// - `fn hash(*Context, T) u64` - Return a hash for an item. This may +/// be generic if `lookupAdapted` is used with another key type. /// -/// - `fn eql(*Context, T, T) bool` - Check two items for equality. +/// - `fn eql(*Context, T, T) bool` - Check two items for equality. This may +/// accept a generic first argument if `lookupAdapted` is used. /// The first of the two items passed in is guaranteed to be from /// a value passed in to an `add` or `lookup` function, the second /// is guaranteed to be a value already resident in the set. @@ -528,6 +530,12 @@ pub fn RefCountedSet( return self.lookupContext(base, value, self.context); } pub fn lookupContext(self: *const Self, base: anytype, value: T, ctx: Context) ?Id { + return self.lookupAdapted(base, value, ctx); + } + + /// Look up a value using a key and context whose hash and equality + /// functions adapt that key to resident values of type T. + pub fn lookupAdapted(self: *const Self, base: anytype, value: anytype, ctx: anytype) ?Id { // A zero-capacity set (a valid special case of Layout.init) // contains nothing and has a zero-size table, so we can't // probe it: table[0] would read whatever memory follows the