Address review comments

1. Switch to using Wyhash instead of a cryptographic hash.
2. Move global variables to App struct.
This commit is contained in:
Jeffrey C. Ollie
2024-05-17 17:13:43 -05:00
parent a89f817b9e
commit 7c893881c3
2 changed files with 25 additions and 14 deletions

View File

@ -45,6 +45,12 @@ quit: bool,
/// same font configuration. /// same font configuration.
font_grid_set: font.SharedGridSet, font_grid_set: font.SharedGridSet,
// Used to rate limit desktop notifications. Some platforms (notably macOS) will
// run out of resources if desktop notifications are sent too fast and the OS
// will kill Ghostty.
last_notification_time: ?std.time.Instant = null,
last_notification_digest: u64 = 0,
/// Initialize the main app instance. This creates the main window, sets /// Initialize the main app instance. This creates the main window, sets
/// up the renderer state, compiles the shaders, etc. This is the primary /// up the renderer state, compiles the shaders, etc. This is the primary
/// "startup" logic. /// "startup" logic.

View File

@ -3486,32 +3486,37 @@ fn completeClipboardReadOSC52(
self.io_thread.wakeup.notify() catch {}; self.io_thread.wakeup.notify() catch {};
} }
const hash_algorithm = std.crypto.hash.sha2.Sha224; // Wyhash is used to hash the contents of the desktop notification to limit how
var last_notification_time: ?std.time.Instant = null; // fast identical notifications can be sent sequentially.
var last_notification_digest = [_]u8{0} ** hash_algorithm.digest_length; const hash_algorithm = std.hash.Wyhash;
// This seed for Wyhash was literally chosen at random. The actual seed (AFAIK)
// shouldn't matter as long as it stays constant.
const hash_seed = 0xb8179c65b93cc558;
fn showDesktopNotification(self: *Surface, title: [:0]const u8, body: [:0]const u8) !void { fn showDesktopNotification(self: *Surface, title: [:0]const u8, body: [:0]const u8) !void {
if (@hasDecl(apprt.Surface, "showDesktopNotification")) { if (@hasDecl(apprt.Surface, "showDesktopNotification")) {
const now = try std.time.Instant.now(); const now = try std.time.Instant.now();
var new_notification_digest: [hash_algorithm.digest_length]u8 = undefined; // Set a limit of one desktop notification per second so that the OS
// doesn't kill us when we run out of resources.
if (last_notification_time) |last| { if (self.app.last_notification_time) |last| {
if (now.since(last) < 1 * std.time.ns_per_s) { if (now.since(last) < 1 * std.time.ns_per_s) {
log.warn("rate limiting desktop notifications", .{}); log.warn("rate limiting desktop notifications", .{});
return; return;
} }
} }
{ const new_notification_digest = d: {
var hash = hash_algorithm.init(.{}); var hash = hash_algorithm.init(hash_seed);
hash.update(title); hash.update(title);
hash.update(body); hash.update(body);
hash.final(&new_notification_digest); break :d hash.final();
} };
if (last_notification_time) |last| { // Set a limit of one notification per five seconds for desktop
if (std.mem.eql(u8, &last_notification_digest, &new_notification_digest)) { // notifications with identical content.
if (self.app.last_notification_time) |last| {
if (self.app.last_notification_digest == new_notification_digest) {
if (now.since(last) < 5 * std.time.ns_per_s) { if (now.since(last) < 5 * std.time.ns_per_s) {
log.warn("suppressing identical desktop notification", .{}); log.warn("suppressing identical desktop notification", .{});
return; return;
@ -3519,8 +3524,8 @@ fn showDesktopNotification(self: *Surface, title: [:0]const u8, body: [:0]const
} }
} }
last_notification_time = now; self.app.last_notification_time = now;
@memcpy(&last_notification_digest, &new_notification_digest); self.app.last_notification_digest = new_notification_digest;
try self.rt_surface.showDesktopNotification(title, body); try self.rt_surface.showDesktopNotification(title, body);
} else log.warn("runtime doesn't support desktop notifications", .{}); } else log.warn("runtime doesn't support desktop notifications", .{});