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.
This commit is contained in:
Mitchell Hashimoto
2024-05-18 10:05:11 -04:00
parent eee58b9ce6
commit d7c64f57b1
2 changed files with 41 additions and 1 deletions

View File

@ -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;

View File

@ -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.