mirrors/ghostty
1
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2026-08-25 00:21:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

terminal/kitty: evict without scratch allocation (#13627)

Browse Source 
Track each image's placement count in its existing metadata. This lets
us use constant-time usage checks (rather than scans) during eviction.

Select the best candidate directly from storage on each eviction,
preserving the existing priority order: unused status, transient hint,
generation, then ID.

Since eviction no longer allocates, it can't fail, so callers no longer
need to handle out-of-memory conditions.
This commit is contained in:
Mitchell Hashimoto
2026-08-05 15:28:41 -07:00
committed by GitHub
parent 090d161b28 4371871bc2
commit 9cb2147641
8 changed files with 155 additions and 161 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/terminal/Screen.zig
View File

@@ -334,14 +334,12 @@ pub fn init(
};
if (comptime build_options.kitty_graphics) {
// This can't fail because the storage is always empty at this point
// and the only fail-able case is that we have to evict images.
result.kitty_images.setLimit(
io,
alloc,
&result,
opts.kitty_image_storage_limit,
) catch unreachable;
);
result.kitty_images.image_limits = opts.kitty_image_loading_limits;
}

4
src/terminal/Terminal.zig
View File

@@ -3560,12 +3560,12 @@ pub fn setKittyGraphicsSizeLimit(
self: *Terminal,
alloc: Allocator,
limit: usize,
) !void {
) void {
if (comptime !build_options.kitty_graphics) return;
var it = self.screens.all.iterator();
while (it.next()) |entry| {
const screen: *Screen = entry.value.*;
try screen.kitty_images.setLimit(self.io(), alloc, screen, limit);
screen.kitty_images.setLimit(self.io(), alloc, screen, limit);
}
}

2
src/terminal/c/terminal.zig
View File

@@ -1008,7 +1008,7 @@ fn setTyped(
var it = wrapper.terminal.screens.all.iterator();
while (it.next()) |entry| {
const screen = entry.value.*;
screen.kitty_images.setLimit(screen.io, screen.alloc, screen, limit) catch return .out_of_memory;
screen.kitty_images.setLimit(screen.io, screen.alloc, screen, limit);
}
},
.kitty_image_medium_file,

6
src/terminal/kitty/graphics_exec.zig
View File

@@ -172,7 +172,7 @@ fn transmit(
// If the loaded image was assigned its ID automatically, not based
// on a number or explicitly specified ID, then we don't respond.
if (load.image.implicit_id) return .{};
if (load.image.metadata.implicit_id) return .{};
// After the image is added, set the ID in case it changed.
// The resulting image number and placement ID never change.
@@ -355,8 +355,8 @@ fn loadAndAddImage(
storage.next_image_id +%= 1;
// If the image also has no number then its auto-ID is "implicit".
// See the doc comment on the Image.implicit_id field for more detail.
if (loading.image.number == 0) loading.image.implicit_id = true;
// See the doc comment on Image.metadata.implicit_id for more detail.
if (loading.image.number == 0) loading.image.metadata.implicit_id = true;
}
// If this is chunked, this is the beginning of a new chunked transmission.

21
src/terminal/kitty/graphics_image.zig
View File

@@ -99,7 +99,7 @@ pub const LoadingImage = struct {
.height = t.height,
.compression = t.compression,
.format = t.format,
.usage = t.usage,
.metadata = .{ .transient = t.usage.transient },
},
.display = cmd.display(),
@@ -600,7 +600,18 @@ pub const Image = struct {
format: command.Transmission.Format = .rgb,
compression: command.Transmission.Compression = .none,
data: Data = .{ .complete = "" },
usage: command.Transmission.Usage = .default,
metadata: packed struct(u32) {
/// The image's transient usage hint, used to prioritize eviction.
transient: bool = false,
/// Set this if the image was loaded without an ID or number. Such
/// images must not receive responses even though they currently get
/// IDs in the public range (which is bad!).
implicit_id: bool = false,
/// Number of placements referencing this image.
placement_count: u30 = 0,
} = .{},
/// Unique, monotonically increasing stamp assigned each time an
/// image is added to (or replaced in) an ImageStorage. A changed
@@ -610,12 +621,6 @@ pub const Image = struct {
/// transmission time. Zero means "never stored".
generation: u64 = 0,
/// Set this to true if this image was loaded by a command that
/// doesn't specify an ID or number, since such commands should
/// not be responded to, even though we do currently give them
/// IDs in the public range (which is bad!).
implicit_id: bool = false,
pub const Error = error{
InvalidData,
DecompressionFailed,

275
src/terminal/kitty/graphics_storage.zig
View File

@@ -171,12 +171,12 @@ pub const ImageStorage = struct {
) void {
if (self.loading) |loading| loading.destroy(alloc);
self.clearPlacements(s);
self.placements.deinit(alloc);
var it = self.images.iterator();
while (it.next()) |kv| kv.value_ptr.deinit(alloc);
self.images.deinit(alloc);
self.clearPlacements(s);
self.placements.deinit(alloc);
}
/// Kitty image protocol is enabled if we have a non-zero limit.
@@ -207,7 +207,7 @@ pub const ImageStorage = struct {
alloc: Allocator,
s: *terminal.Screen,
limit: usize,
) !void {
) void {
// Special case disabling by quickly deleting all
if (limit == 0) {
const image_limits = self.image_limits;
@@ -220,7 +220,7 @@ pub const ImageStorage = struct {
if (limit < self.total_bytes) {
const req_bytes = self.total_bytes - limit;
log.info("evicting images to lower limit, evicting={}", .{req_bytes});
if (!try self.evictImage(io, alloc, req_bytes)) {
if (!self.evictImage(io, alloc, req_bytes)) {
log.warn("failed to evict enough images for required bytes", .{});
}
}
@@ -258,7 +258,7 @@ pub const ImageStorage = struct {
if (total_bytes > self.total_limit) {
const req_bytes = total_bytes - self.total_limit;
log.info("evicting images to make space for {} bytes", .{req_bytes});
if (!try self.evictImageExcept(io, alloc, req_bytes, img.id)) {
if (!self.evictImageExcept(io, alloc, req_bytes, img.id)) {
log.warn("failed to evict enough images for required bytes", .{});
return error.OutOfMemory;
}
@@ -268,13 +268,16 @@ pub const ImageStorage = struct {
log.debug("addImage image={}", .{img.withoutData()});
// Write our new image
if (gop.found_existing) {
// Write our new image, preserving placements across replacement.
const placement_count = if (gop.found_existing) count: {
self.total_bytes -= gop.value_ptr.data.len();
const count = gop.value_ptr.metadata.placement_count;
gop.value_ptr.deinit(alloc);
}
break :count count;
} else 0;
gop.value_ptr.* = img;
gop.value_ptr.metadata.placement_count = placement_count;
self.total_bytes += new_len;
// Stamp the stored image with a fresh generation. This gives
@@ -347,7 +350,19 @@ pub const ImageStorage = struct {
};
const gop = try self.placements.getOrPut(alloc, key);
if (gop.found_existing) gop.value_ptr.deinit(s);
if (gop.found_existing) {
gop.value_ptr.deinit(s);
} else {
const img = self.images.getPtr(image_id).?;
img.metadata.placement_count = std.math.add(
@TypeOf(img.metadata.placement_count),
img.metadata.placement_count,
1,
) catch {
self.placements.removeByPtr(gop.key_ptr);
return error.OutOfMemory;
};
}
gop.value_ptr.* = p;
self.markMutated(io);
@@ -369,7 +384,7 @@ pub const ImageStorage = struct {
if (!pin.garbage) continue;
entry.value_ptr.deinit(s);
self.placements.removeByPtr(entry.key_ptr);
self.removePlacementByPtr(entry.key_ptr);
removed = true;
}
@@ -382,6 +397,13 @@ pub const ImageStorage = struct {
self.placements.clearRetainingCapacity();
}
fn removePlacementByPtr(self: *ImageStorage, key: *PlacementKey) void {
const img = self.images.getPtr(key.image_id).?;
assert(img.metadata.placement_count > 0);
img.metadata.placement_count -= 1;
self.placements.removeByPtr(key);
}
/// Get an image by its ID. If the image doesn't exist, null is returned.
pub fn imageById(self: *const ImageStorage, image_id: u32) ?Image {
return self.images.get(image_id);
@@ -437,7 +459,7 @@ pub const ImageStorage = struct {
// Deinit the placement and remove it
const image_id = entry.key_ptr.image_id;
entry.value_ptr.deinit(t.screens.active);
self.placements.removeByPtr(entry.key_ptr);
self.removePlacementByPtr(entry.key_ptr);
if (delete_images) self.deleteIfUnused(alloc, image_id);
}
@@ -535,7 +557,7 @@ pub const ImageStorage = struct {
const rect = entry.value_ptr.rect(img, t) orelse continue;
if (rect.top_left.x <= x and rect.bottom_right.x >= x) {
entry.value_ptr.deinit(t.screens.active);
self.placements.removeByPtr(entry.key_ptr);
self.removePlacementByPtr(entry.key_ptr);
if (v.delete) self.deleteIfUnused(alloc, img.id);
}
}
@@ -564,7 +586,7 @@ pub const ImageStorage = struct {
target_pin_copy.x = rect.top_left.x;
if (target_pin_copy.isBetween(rect.top_left, rect.bottom_right)) {
entry.value_ptr.deinit(t.screens.active);
self.placements.removeByPtr(entry.key_ptr);
self.removePlacementByPtr(entry.key_ptr);
if (v.delete) self.deleteIfUnused(alloc, img.id);
}
}
@@ -584,7 +606,7 @@ pub const ImageStorage = struct {
if (entry.value_ptr.z == v.z) {
const image_id = entry.key_ptr.image_id;
entry.value_ptr.deinit(t.screens.active);
self.placements.removeByPtr(entry.key_ptr);
self.removePlacementByPtr(entry.key_ptr);
if (v.delete) self.deleteIfUnused(alloc, image_id);
}
}
@@ -605,7 +627,7 @@ pub const ImageStorage = struct {
if (entry.key_ptr.image_id >= v.first and entry.key_ptr.image_id <= v.last) {
const image_id = entry.key_ptr.image_id;
entry.value_ptr.deinit(t.screens.active);
self.placements.removeByPtr(entry.key_ptr);
self.removePlacementByPtr(entry.key_ptr);
if (v.delete) self.deleteIfUnused(alloc, image_id);
}
}
@@ -631,7 +653,7 @@ pub const ImageStorage = struct {
while (it.next()) |entry| {
if (entry.key_ptr.image_id == image_id) {
entry.value_ptr.deinit(s);
self.placements.removeByPtr(entry.key_ptr);
self.removePlacementByPtr(entry.key_ptr);
}
}
} else {
@@ -640,7 +662,7 @@ pub const ImageStorage = struct {
.placement_id = .{ .tag = .external, .id = placement_id },
})) |entry| {
entry.value_ptr.deinit(s);
self.placements.removeByPtr(entry.key_ptr);
self.removePlacementByPtr(entry.key_ptr);
}
}
@@ -651,19 +673,12 @@ pub const ImageStorage = struct {
/// Delete an image if it is unused.
fn deleteIfUnused(self: *ImageStorage, alloc: Allocator, image_id: u32) void {
var it = self.placements.iterator();
while (it.next()) |kv| {
if (kv.key_ptr.image_id == image_id) {
return;
}
}
const entry = self.images.getEntry(image_id) orelse return;
if (entry.value_ptr.metadata.placement_count > 0) return;
// If we get here, we can delete the image.
if (self.images.getEntry(image_id)) |entry| {
self.total_bytes -= entry.value_ptr.data.len();
entry.value_ptr.deinit(alloc);
self.images.removeByPtr(entry.key_ptr);
}
self.total_bytes -= entry.value_ptr.data.len();
entry.value_ptr.deinit(alloc);
self.images.removeByPtr(entry.key_ptr);
}
/// Deletes all placements intersecting a screen point.
@@ -686,7 +701,7 @@ pub const ImageStorage = struct {
if (rect.contains(target_pin)) {
if (filter) |f| if (!f(filter_ctx, entry.value_ptr.*)) continue;
entry.value_ptr.deinit(t.screens.active);
self.placements.removeByPtr(entry.key_ptr);
self.removePlacementByPtr(entry.key_ptr);
if (delete_unused) self.deleteIfUnused(alloc, img.id);
}
}
@@ -698,7 +713,7 @@ pub const ImageStorage = struct {
///
/// This will evict as many images as necessary to make space for
/// req bytes.
fn evictImage(self: *ImageStorage, io: std.Io, alloc: Allocator, req: usize) !bool {
fn evictImage(self: *ImageStorage, io: std.Io, alloc: Allocator, req: usize) bool {
return self.evictImageExcept(io, alloc, req, null);
}
@@ -710,128 +725,77 @@ pub const ImageStorage = struct {
alloc: Allocator,
req: usize,
exclude_id: ?u32,
) !bool {
) bool {
assert(req <= self.total_limit);
// Ironically we allocate to evict. We should probably redesign the
// data structures to avoid this but for now allocating a little
// bit is fine compared to the megabytes we're looking to save.
const Candidate = struct {
id: u32,
generation: u64,
// Map images into four distinct blocks:
// Lower values are evicted first:
// 0: transient, unused
// 1: not transient, unused
// 2: transient, used
// 3: not transient, used
block: u2,
priority: u2,
fn init(img: *const Image) @This() {
return .{
.id = img.id,
.generation = img.generation,
.priority = (if (img.metadata.transient) @as(u2, 0) else @as(u2, 1)) +
(if (img.metadata.placement_count > 0) @as(u2, 2) else @as(u2, 0)),
};
}
fn lessThan(lhs: @This(), rhs: @This()) bool {
if (lhs.priority != rhs.priority)
return lhs.priority < rhs.priority;
if (lhs.generation != rhs.generation)
return lhs.generation < rhs.generation;
return lhs.id < rhs.id;
}
};
const candidates = try alloc.alloc(Candidate, self.images.count());
defer alloc.free(candidates);
var it = self.images.iterator();
var candidate_count: usize = 0;
while (it.next()) |kv| {
const img = kv.value_ptr;
if (exclude_id != null and img.id == exclude_id.?) continue;
// This is a huge waste. See comment above about redesigning
// our data structures to avoid this. Eviction should be very
// rare though and we never have that many images/placements
// so hopefully this will last a long time.
const used = used: {
var p_it = self.placements.iterator();
while (p_it.next()) |p_kv| {
if (p_kv.key_ptr.image_id == img.id) {
break :used true;
}
}
break :used false;
};
const transient = img.usage.transient;
candidates[candidate_count] = .{
.id = img.id,
.generation = img.generation,
// Map images into four distinct blocks:
// 0: transient, unused
// 1: not transient, unused
// 2: transient, used
// 3: not transient, used
.block = (if (transient) @as(u2, 0) else @as(u2, 1)) + (if (used) @as(u2, 2) else @as(u2, 0)),
};
candidate_count += 1;
}
const candidate_slice = candidates[0..candidate_count];
// Sort
std.mem.sortUnstable(
Candidate,
candidate_slice,
{},
struct {
fn lessThan(
ctx: void,
lhs: Candidate,
rhs: Candidate,
) bool {
_ = ctx;
// If images mapped into different blocks, prioritize lower
// numbered blocks.
if (lhs.block < rhs.block) return true;
if (lhs.block > rhs.block) return false;
// If images mapped to the same block, compare generations.
return if (lhs.generation == rhs.generation)
// If the generation is the same, use the ID to
// prioritize evicting "earlier" images.
lhs.id < rhs.id
else
// If the generation is different, prioritize evicting
// images from earlied generations.
lhs.generation < rhs.generation;
}
}.lessThan,
);
// Evicting anything is a content mutation. This matters for the
// setLimit path in particular, which doesn't otherwise mark it.
var any_evicted = false;
defer if (any_evicted) self.markMutated(io);
const images_before = self.images.count();
defer if (self.images.count() != images_before) self.markMutated(io);
// They're in order of best to evict.
var evicted: usize = 0;
for (candidate_slice) |c| {
// Delete all the placements for this image and the image.
while (evicted < req) {
const c = candidate: {
var best: ?Candidate = null;
var it = self.images.iterator();
while (it.next()) |kv| {
const img = kv.value_ptr;
if (exclude_id != null and img.id == exclude_id.?) continue;
const current = Candidate.init(img);
if (best == null or current.lessThan(best.?)) best = current;
}
break :candidate best orelse return false;
};
var p_it = self.placements.iterator();
while (p_it.next()) |entry| {
if (entry.key_ptr.image_id == c.id) {
self.placements.removeByPtr(entry.key_ptr);
any_evicted = true;
self.removePlacementByPtr(entry.key_ptr);
}
}
if (self.images.getEntry(c.id)) |entry| {
const image_len = entry.value_ptr.data.len();
log.info("evicting image id={} bytes={}", .{ c.id, image_len });
const entry = self.images.getEntry(c.id).?;
const image_len = entry.value_ptr.data.len();
log.info("evicting image id={} bytes={}", .{ c.id, image_len });
evicted += image_len;
self.total_bytes -= image_len;
evicted += image_len;
self.total_bytes -= image_len;
entry.value_ptr.deinit(alloc);
self.images.removeByPtr(entry.key_ptr);
any_evicted = true;
if (evicted >= req) return true;
}
entry.value_ptr.deinit(alloc);
self.images.removeByPtr(entry.key_ptr);
}
return false;
return true;
}
/// Every placement is uniquely identified by the image ID and the
@@ -1162,6 +1126,28 @@ test "storage: adding placement reclaims garbage placements" {
);
}
test "storage: placement count limit permits replacement" {
const testing = std.testing;
const io = testing.io;
const alloc = testing.allocator;
var t = try terminal.Terminal.init(io, alloc, .{ .rows = 3, .cols = 3 });
defer t.deinit(alloc);
var s: ImageStorage = .{};
defer s.deinit(alloc, t.screens.active);
try s.addImage(io, alloc, .{ .id = 1 });
try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .virtual = {} } });
const img = s.images.getPtr(1).?;
img.metadata.placement_count = std.math.maxInt(@TypeOf(img.metadata.placement_count));
try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .virtual = {} } });
try testing.expectError(
error.OutOfMemory,
s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .virtual = {} } }),
);
try testing.expectEqual(@as(usize, 1), s.placements.count());
}
test "storage: delete all placements and images" {
const testing = std.testing;
const alloc = testing.allocator;
@@ -1957,7 +1943,7 @@ test "storage: generation bumps when setLimit evicts or disables" {
// Lowering the limit evicts the image and must mark a mutation.
s.dirty = false;
try s.setLimit(io, alloc, t.screens.active, 1);
s.setLimit(io, alloc, t.screens.active, 1);
try testing.expect(s.dirty);
try testing.expect(s.generation > gen_add);
try testing.expectEqual(@as(usize, 0), s.images.count());
@@ -1965,7 +1951,7 @@ test "storage: generation bumps when setLimit evicts or disables" {
// Disabling (limit=0) resets the storage and must mark a mutation.
s.dirty = false;
try s.setLimit(io, alloc, t.screens.active, 0);
s.setLimit(io, alloc, t.screens.active, 0);
try testing.expect(s.dirty);
try testing.expect(s.generation > gen_evict);
}
@@ -2030,7 +2016,7 @@ test "storage: no-op delete does not mark a mutation" {
try testing.expect(s.generation > gen);
}
test "storage: evict unused transient image" {
test "storage: evicts images in priority order" {
const testing = std.testing;
const io = testing.io;
const alloc = testing.allocator;
@@ -2043,17 +2029,17 @@ test "storage: evict unused transient image" {
try s.addImage(io, alloc, .{
.id = 1,
.data = .{ .complete = try alloc.dupe(u8, "*" ** 64) },
.usage = .{ .transient = false },
.metadata = .{ .transient = false },
});
try s.addImage(io, alloc, .{
.id = 2,
.data = .{ .complete = try alloc.dupe(u8, "*" ** 64) },
.usage = .{ .transient = true },
.metadata = .{ .transient = true },
});
try s.addImage(io, alloc, .{
.id = 3,
.data = .{ .complete = try alloc.dupe(u8, "*" ** 64) },
.usage = .{ .transient = true },
.metadata = .{ .transient = true },
});
try s.addPlacement(
io,
@@ -2065,12 +2051,12 @@ test "storage: evict unused transient image" {
);
const gen = s.generation;
const result = try s.evictImage(io, alloc, 32);
const result = s.evictImage(io, alloc, 96);
try testing.expect(s.dirty);
try testing.expect(s.generation > gen);
try testing.expectEqual(true, result);
try testing.expectEqual(2, s.images.count());
try testing.expect(s.images.contains(1));
try testing.expectEqual(1, s.images.count());
try testing.expect(!s.images.contains(1));
try testing.expect(s.images.contains(2));
try testing.expect(!s.images.contains(3));
}
@@ -2224,6 +2210,11 @@ test "storage: replacement reuses pending reservation and preserves placements"
try testing.expect(!s.images.contains(2));
try testing.expectEqual(@as(usize, 1), s.placements.count());
try testing.expectEqual(@as(usize, 10), s.total_bytes);
// The placement remains a live reference across both replacements.
s.delete(io, alloc, &t, .{ .id = .{ .delete = true, .image_id = 1 } });
try testing.expectEqual(@as(usize, 0), s.images.count());
try testing.expectEqual(@as(usize, 0), s.placements.count());
}
test "storage: pending images share exact eviction ordering" {
@@ -2239,24 +2230,24 @@ test "storage: pending images share exact eviction ordering" {
_ = try s.addPendingImage(io, alloc, .{
.id = 1,
.data = .{ .pending = 64 },
.usage = .{ .transient = false },
.metadata = .{ .transient = false },
});
_ = try s.addPendingImage(io, alloc, .{
.id = 2,
.data = .{ .pending = 64 },
.usage = .{ .transient = true },
.metadata = .{ .transient = true },
});
try s.addImage(io, alloc, .{
.id = 3,
.data = .{ .complete = try alloc.dupe(u8, "*" ** 64) },
.usage = .{ .transient = true },
.metadata = .{ .transient = true },
});
try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{
.location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) },
});
// ID 3 is transient and unused, so one exact-size eviction is enough.
try testing.expect(try s.evictImage(io, alloc, 64));
try testing.expect(s.evictImage(io, alloc, 64));
try testing.expect(s.images.contains(1));
try testing.expect(s.images.contains(2));
try testing.expect(!s.images.contains(3));

2
src/terminal/snapshot/screen.zig
View File

@@ -505,7 +505,7 @@ pub fn decode(
alloc,
&result,
options.kitty_image_storage_limit,
) catch unreachable;
);
result.kitty_images.image_limits = options.kitty_image_loading_limits;
}

2
src/termio/Termio.zig
View File

@@ -460,7 +460,7 @@ pub fn changeConfig(self: *Termio, td: *ThreadData, config: *DerivedConfig) !voi
};
// Set the image limits
try self.terminal.setKittyGraphicsSizeLimit(self.alloc, config.image_storage_limit);
self.terminal.setKittyGraphicsSizeLimit(self.alloc, config.image_storage_limit);
self.terminal.setKittyGraphicsLoadingLimits(.allWithTempDir(global.tmpDirPath()));
}