From d7c64f57b1860cd67938ea1e321ae08045e30b77 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Sat, 18 May 2024 10:05:11 -0400
Subject: [PATCH] font/shaper: periodically reset LRU in cache to avoid
 slowdown

See: https://github.com/ziglang/zig/issues/17851

Users were noticing that frame render times got slower over time. I
believe (thanks to community for pointing it out) that this is the
culprit.

This works around this issue by clearing and reinitializing the LRU
after a certain number of evictions. When the Zig issue has a better
resolution (either rehash() as a workaround or a better hash
implementation overall) we can change this.
---
 src/font/shaper/Cache.zig | 35 ++++++++++++++++++++++++++++++++++-
 src/lru.zig               |  7 +++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/font/shaper/Cache.zig b/src/font/shaper/Cache.zig
index 9da736621c..afb89ee9c8 100644
--- a/src/font/shaper/Cache.zig
+++ b/src/font/shaper/Cache.zig
@@ -21,9 +21,25 @@ const log = std.log.scoped(.font_shaper_cache);
 /// Our LRU is the run hash to the shaped cells.
 const LRU = lru.AutoHashMap(u64, []font.shape.Cell);
 
+/// This is the threshold of evictions at which point we reset
+/// the LRU completely. This is a workaround for the issue that
+/// Zig stdlib hashmap gets slower over time
+/// (https://github.com/ziglang/zig/issues/17851).
+///
+/// The value is based on naive measuring on my local machine.
+/// If someone has a better idea of what this value should be,
+/// please let me know.
+const evictions_threshold = 8192;
+
 /// The cache of shaped cells.
 map: LRU,
 
+/// Keep track of the number of evictions. We use this to workaround
+/// the issue that Zig stdlib hashmap gets slower over time
+/// (https://github.com/ziglang/zig/issues/17851). When evictions
+/// reaches a certain threshold, we reset the LRU.
+evictions: std.math.IntFittingRange(0, evictions_threshold) = 0,
+
 pub fn init() Cache {
     // Note: this is very arbitrary. Increasing this number will increase
     // the cache hit rate, but also increase the memory usage. We should do
@@ -56,8 +72,20 @@ pub fn put(
     const copy = try alloc.dupe(font.shape.Cell, cells);
     const gop = try self.map.getOrPut(alloc, run.hash);
     if (gop.evicted) |evicted| {
-        log.debug("evicted shaped cells for text run hash={}", .{run.hash});
         alloc.free(evicted.value);
+
+        // See the doc comment on evictions_threshold for why we do this.
+        self.evictions += 1;
+        if (self.evictions >= evictions_threshold) {
+            log.debug("resetting cache due to too many evictions", .{});
+            // We need to put our value here so deinit can free
+            gop.value_ptr.* = copy;
+            self.clear(alloc);
+
+            // We need to call put again because self is now a
+            // different pointer value so our gop pointers are invalid.
+            return try self.put(alloc, run, cells);
+        }
     }
     gop.value_ptr.* = copy;
 }
@@ -66,6 +94,11 @@ pub fn count(self: *const Cache) usize {
     return self.map.map.count();
 }
 
+fn clear(self: *Cache, alloc: Allocator) void {
+    self.deinit(alloc);
+    self.* = init();
+}
+
 test Cache {
     const testing = std.testing;
     const alloc = testing.allocator;
diff --git a/src/lru.zig b/src/lru.zig
index bb7977f00c..f303158e51 100644
--- a/src/lru.zig
+++ b/src/lru.zig
@@ -15,6 +15,13 @@ pub fn AutoHashMap(comptime K: type, comptime V: type) type {
 
 /// HashMap implementation that supports least-recently-used eviction.
 ///
+/// Beware of the Zig bug where a hashmap gets slower over time
+/// (https://github.com/ziglang/zig/issues/17851). This LRU uses a hashmap
+/// and evictions will cause this issue to appear. Callers should keep
+/// track of eviction counts and periodically reinitialize the LRU to
+/// avoid this issue. The LRU itself can't do this because it doesn't
+/// know how to free values.
+///
 /// Note: This is a really elementary CS101 version of an LRU right now.
 /// This is done initially to get something working. Once we have it working,
 /// we can benchmark and improve if this ends up being a source of slowness.