Skip to content

Commit

Permalink
Merge pull request #1777 from mitchellh/lru-slow
Browse files Browse the repository at this point in the history
font/shaper: periodically reset LRU in cache to avoid slowdown
  • Loading branch information
mitchellh authored May 18, 2024
2 parents eee58b9 + d7c64f5 commit f72c2ac
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
35 changes: 34 additions & 1 deletion src/font/shaper/Cache.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions src/lru.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit f72c2ac

Please sign in to comment.