From de92470f0d9674aed1916e7d028f59c5e24d20a4 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Tue, 13 Feb 2024 11:00:40 +0530 Subject: [PATCH] Improve performance of disk cache when there are thousands of small images Fixes #7080 --- kitty/disk-cache.c | 103 ++++++++++++++++++++++++++++++++-------- kitty_tests/graphics.py | 30 ++++++++++++ 2 files changed, 112 insertions(+), 21 deletions(-) diff --git a/kitty/disk-cache.c b/kitty/disk-cache.c index f84c98754..bbb01dc5d 100644 --- a/kitty/disk-cache.c +++ b/kitty/disk-cache.c @@ -7,6 +7,7 @@ #define EXTRA_INIT if (PyModule_AddFunctions(module, module_methods) != 0) return false; #define MAX_KEY_SIZE 256u + #include "disk-cache.h" #include "safe-wrappers.h" #include "kitty-uthash.h" @@ -32,16 +33,27 @@ typedef struct { UT_hash_handle hh; } CacheEntry; +typedef struct Hole { + off_t pos, size; +} Hole; + +typedef struct Holes { + Hole *items; + size_t capacity, count; + off_t largest_hole_size; +} Holes; typedef struct { PyObject_HEAD char *cache_dir; int cache_file_fd; + Py_ssize_t small_hole_threshold; pthread_mutex_t lock; pthread_t write_thread; bool thread_started, lock_inited, loop_data_inited, shutting_down, fully_initialized; LoopData loop_data; CacheEntry *entries, currently_writing; + Holes holes; unsigned long long total_size; } DiskCache; @@ -71,6 +83,7 @@ new_diskcache_object(PyTypeObject *type, PyObject UNUSED *args, PyObject UNUSED self = (DiskCache*)type->tp_alloc(type, 0); if (self) { self->cache_file_fd = -1; + self->small_hole_threshold = 512; } return (PyObject*) self; } @@ -177,6 +190,8 @@ defrag(DiskCache *self) { e->new_offset = current_pos; current_pos += e->data_sz; } + self->holes.count = 0; + self->holes.largest_hole_size = 0; ok = true; cleanup: @@ -194,24 +209,66 @@ cleanup: if (new_cache_file > -1) safe_close(new_cache_file, __FILE__, __LINE__); } -static int -cmp_pos_in_cache_file(void *a_, void *b_) { - CacheEntry *a = a_, *b = b_; - return a->pos_in_cache_file - b->pos_in_cache_file; +static bool +find_hole_to_use(DiskCache *self, const off_t required_sz) { + if (self->holes.largest_hole_size < required_sz) return false; + off_t largest_hole_size = 0; + bool found = false; + ssize_t remove_at = -1; + for (size_t i = 0; i < self->holes.count; i++) { + Hole *h = self->holes.items + i; + if (!found && h->size >= required_sz) { + found = true; + self->currently_writing.pos_in_cache_file = h->pos; + bool was_largest_hole = h->size == self->holes.largest_hole_size; + h->size -= required_sz; + h->pos += required_sz; + if (h->size <= self->small_hole_threshold) remove_at = i; + if (!was_largest_hole) { + if (remove_at < 0) return found; + largest_hole_size = self->holes.largest_hole_size; + break; + } + } + if (h->size > largest_hole_size) largest_hole_size = h->size; + } + self->holes.largest_hole_size = largest_hole_size; + if (remove_at > -1) remove_i_from_array(self->holes.items, (size_t)remove_at, self->holes.count); + return found; +} + +static inline bool +needs_defrag(DiskCache *self) { + off_t size_on_disk = size_of_cache_file(self); + return self->total_size && size_on_disk > 0 && (size_t)size_on_disk > self->total_size * 2; } static void -find_hole(DiskCache *self) { - off_t required_size = self->currently_writing.data_sz, prev = -100; - HASH_SORT(self->entries, cmp_pos_in_cache_file); - CacheEntry *s, *tmp; - HASH_ITER(hh, self->entries, s, tmp) { - if (s->pos_in_cache_file >= 0 && s->data_sz > 0) { - if (prev >= 0 && s->pos_in_cache_file - prev >= required_size) { - self->currently_writing.pos_in_cache_file = prev; - return; - } - prev = s->pos_in_cache_file + s->data_sz; +add_hole(DiskCache *self, off_t pos, off_t size) { + if (size <= self->small_hole_threshold) return; + // see if we can find a hole that ends where we start and merge + // look through the prev at most 128 holes for a match + Hole *h = &self->holes.items[self->holes.count-1]; + for (size_t i = 0; i < MIN(self->holes.count, 128u); i++, h--) { + if (h->pos + h->size == pos) { + h->size += size; + self->holes.largest_hole_size = MAX(self->holes.largest_hole_size, h->size); + return; + } + } + ensure_space_for(&self->holes, items, Hole, 1, capacity, 64, false); + h = &self->holes.items[self->holes.count++]; + h->pos = pos; h->size = size; + self->holes.largest_hole_size = MAX(self->holes.largest_hole_size, h->size); +} + +static void +remove_from_disk(DiskCache *self, CacheEntry *s) { + if (s->written_to_disk) { + s->written_to_disk = false; + if (s->data_sz && s->pos_in_cache_file > -1) { + add_hole(self, s->pos_in_cache_file, s->data_sz); + s->pos_in_cache_file = -1; } } } @@ -219,8 +276,7 @@ find_hole(DiskCache *self) { static bool find_cache_entry_to_write(DiskCache *self) { CacheEntry *tmp, *s; - off_t size_on_disk = size_of_cache_file(self); - if (self->total_size && size_on_disk > 0 && (size_t)size_on_disk > self->total_size * 2) defrag(self); + if (needs_defrag(self)) defrag(self); HASH_ITER(hh, self->entries, s, tmp) { if (!s->written_to_disk) { if (s->data) { @@ -232,7 +288,7 @@ find_cache_entry_to_write(DiskCache *self) { xor_data(s->encryption_key, sizeof(s->encryption_key), self->currently_writing.data, s->data_sz); self->currently_writing.hash_keylen = MIN(s->hash_keylen, MAX_KEY_SIZE); memcpy(self->currently_writing.hash_key, s->hash_key, self->currently_writing.hash_keylen); - find_hole(self); + find_hole_to_use(self, self->currently_writing.data_sz); return true; } s->written_to_disk = true; @@ -417,6 +473,7 @@ dealloc(DiskCache* self) { safe_close(self->cache_file_fd, __FILE__, __LINE__); self->cache_file_fd = -1; } + free(self->holes.items); zero_at_ptr(&self->holes); if (self->currently_writing.data) free(self->currently_writing.data); free(self->cache_dir); self->cache_dir = NULL; Py_TYPE(self)->tp_free((PyObject*)self); @@ -451,10 +508,9 @@ add_to_disk_cache(PyObject *self_, const void *key, size_t key_sz, const void *d if (!(s = create_cache_entry(key, key_sz))) goto end; HASH_ADD_KEYPTR(hh, self->entries, s->hash_key, s->hash_keylen, s); } else { - s->written_to_disk = false; + remove_from_disk(self, s); + self->total_size -= MIN(self->total_size, s->data_sz); if (s->data) free(s->data); - if (data_sz <= self->total_size) self->total_size -= data_sz; - else self->total_size = 0; } s->data = copied_data; s->data_sz = data_sz; copied_data = NULL; self->total_size += s->data_sz; @@ -478,6 +534,7 @@ remove_from_disk_cache(PyObject *self_, const void *key, size_t key_sz) { if (s) { removed = true; HASH_DEL(self->entries, s); + remove_from_disk(self, s); self->total_size = (self->total_size > s->data_sz) ? self->total_size - s->data_sz : 0; free_cache_entry(s); } @@ -497,6 +554,9 @@ clear_disk_cache(PyObject *self_) { free_cache_entry(s); } self->total_size = 0; + self->holes.count = 0; + self->holes.largest_hole_size = 0; + if (self->cache_file_fd > -1) add_hole(self, 0, size_of_cache_file(self)); mutex(unlock); wakeup_write_loop(self); } @@ -770,6 +830,7 @@ static PyMethodDef methods[] = { static PyMemberDef members[] = { {"total_size", T_ULONGLONG, offsetof(DiskCache, total_size), READONLY, "total_size"}, + {"small_hole_threshold", T_PYSSIZET, offsetof(DiskCache, small_hole_threshold), 0, "small_hole_threshold"}, {NULL}, }; diff --git a/kitty_tests/graphics.py b/kitty_tests/graphics.py index d6ab2026e..8d16e191f 100644 --- a/kitty_tests/graphics.py +++ b/kitty_tests/graphics.py @@ -199,6 +199,7 @@ def xor(skey, data): def test_disk_cache(self): s = self.create_screen() dc = s.grman.disk_cache + dc.small_hole_threshold = 0 data = {} def key_as_bytes(key): @@ -222,6 +223,13 @@ def check_data(): for key, val in data.items(): self.ae(dc.get(key_as_bytes(key)), val) + def reset(small_hole_threshold=0): + nonlocal dc, data, s + s = self.create_screen() + dc = s.grman.disk_cache + dc.small_hole_threshold = small_hole_threshold + data = {} + for i in range(25): self.assertIsNone(add(i, f'{i}' * i)) @@ -235,6 +243,7 @@ def check_data(): check_data() self.assertRaises(KeyError, dc.get, key_as_bytes(x)) self.assertEqual(sz, dc.size_on_disk()) + self.assertEqual(sz, dc.size_on_disk()) for x in ('xy', 'C'*4, 'B'*6, 'A'*8): add(x, x) self.assertTrue(dc.wait_for_write()) @@ -288,6 +297,27 @@ def clear_predicate(key): dc.remove_from_ram(clear_predicate) self.assertEqual(dc.num_cached_in_ram(), 0) + reset(512) + self.assertIsNone(add(1, '1' * 1024)) + self.assertIsNone(add(2, '2' * 1024)) + dc.wait_for_write() + sz = dc.size_on_disk() + remove(1) + self.ae(sz, dc.size_on_disk()) + self.assertIsNone(add(3, '3' * 800)) + dc.wait_for_write() + self.ae(sz, dc.size_on_disk()) + self.assertIsNone(add(4, '4' * 100)) + sz += 100 + dc.wait_for_write() + self.ae(sz, dc.size_on_disk()) + check_data() + remove(4) + self.assertIsNone(add(5, '5' * 10)) + sz += 10 + dc.wait_for_write() + self.ae(sz, dc.size_on_disk()) + def test_suppressing_gr_command_responses(self): s, g, pl, sl = load_helpers(self) self.ae(pl('abcd', s=10, v=10, q=1), 'ENODATA:Insufficient image data: 4 < 400')