From 2955c3f7a7243b72ec15d79dbfa22a2b2546950e Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Tue, 9 Jul 2024 15:11:54 +0530 Subject: [PATCH] Move hyperlink hashmap to verstable Also speedup getting url for hyperlink id by using a second hash map for it. Change behavior when there are too many hyperlinks to discard new hyperlinks instead of remapping old hyperlink id and making some old hyperlink wrong. --- kitty/hyperlink.c | 187 ++++++++++++++++++++++-------------------- kitty/hyperlink.h | 3 +- kitty/screen.c | 30 +------ kitty_tests/screen.py | 24 +++--- 4 files changed, 116 insertions(+), 128 deletions(-) diff --git a/kitty/hyperlink.c b/kitty/hyperlink.c index 9478927fa..21bc2b63a 100644 --- a/kitty/hyperlink.c +++ b/kitty/hyperlink.c @@ -6,56 +6,47 @@ */ #include "hyperlink.h" -#include "kitty-uthash.h" +#include "lineops.h" #include #define MAX_KEY_LEN 2048 #define MAX_ID_LEN 256 -#define MAX_ADDS_BEFORE_GC 256 -typedef struct { - const char *key; - hyperlink_id_type id; - UT_hash_handle hh; -} HyperLinkEntry; +#define NAME hyperlink_map +#define KEY_TY const char* +#define VAL_TY hyperlink_id_type +#include "kitty-verstable.h" + +#define NAME hyperlink_id_map +#define KEY_TY hyperlink_id_type +#define VAL_TY const char* +#include "kitty-verstable.h" typedef struct { - HyperLinkEntry *hyperlinks; - unsigned int max_link_id, num_of_adds_since_garbage_collection; + hyperlink_map map; + hyperlink_id_map idmap; + hyperlink_id_type max_link_id, num_of_adds_since_garbage_collection; } HyperLinkPool; - -static void -free_hyperlink_entry(HyperLinkEntry *s) { - free((void*)s->key); - free(s); -} - static void clear_pool(HyperLinkPool *pool) { - if (pool->hyperlinks) { - HyperLinkEntry *tmp, *s; - HASH_ITER(hh, pool->hyperlinks, s, tmp) { - HASH_DEL(pool->hyperlinks, s); - free_hyperlink_entry(s); s = NULL; - } - pool->max_link_id = 0; - } + for (hyperlink_map_itr i = vt_first(&pool->map); !vt_is_end(i); i = vt_next(i)) free((char*)i.data->key); + vt_cleanup(&pool->map); vt_cleanup(&pool->idmap); + pool->max_link_id = 0; pool->num_of_adds_since_garbage_collection = 0; } HYPERLINK_POOL_HANDLE alloc_hyperlink_pool(void) { - return calloc(1, sizeof(HyperLinkPool)); + HyperLinkPool *ans = calloc(1, sizeof(HyperLinkPool)); + if (ans) { vt_init(&ans->map); vt_init(&ans->idmap); } + return (HYPERLINK_POOL_HANDLE)ans; } void clear_hyperlink_pool(HYPERLINK_POOL_HANDLE h) { - if (h) { - HyperLinkPool *pool = (HyperLinkPool*)h; - clear_pool(pool); - } + if (h) clear_pool((HyperLinkPool*)h); } @@ -68,29 +59,68 @@ free_hyperlink_pool(HYPERLINK_POOL_HANDLE h) { } } +#define NAME id_id_map +#define KEY_TY hyperlink_id_type +#define VAL_TY hyperlink_id_type +#include "kitty-verstable.h" + + +static const char* +dupstr(const char *src, size_t len) { + char *ans = malloc(len+1); + if (!ans) fatal("Out of memory"); + memcpy(ans, src, len); ans[len] = 0; + return ans; +} + +static void +process_cell(HyperLinkPool *pool, id_id_map *map, hyperlink_id_map *clone, CPUCell *c) { + if (!c->hyperlink_id) return; + id_id_map_itr n = vt_get(map, c->hyperlink_id); + hyperlink_id_type new_id; + if (vt_is_end(n)) { + hyperlink_id_map_itr i = vt_get(clone, c->hyperlink_id); + if (vt_is_end(i)) new_id = 0; + else { + new_id = ++pool->max_link_id; + if (vt_is_end(vt_insert(map, c->hyperlink_id, new_id))) fatal("Out of memory"); + const char *key = i.data->val; + if (vt_is_end(vt_insert(&pool->map, key, new_id))) fatal("Out of memory"); + if (vt_is_end(vt_insert(&pool->idmap, new_id, key))) fatal("Out of memory"); + vt_erase_itr(clone, i); + } + } else new_id = n.data->val; + c->hyperlink_id = new_id; +} + +static void +remap_hyperlink_ids(Screen *self, id_id_map *map, hyperlink_id_map *clone) { + HyperLinkPool *pool = (HyperLinkPool*)self->hyperlink_pool; + if (self->historybuf->count) { + for (index_type y = self->historybuf->count; y-- > 0;) { + CPUCell *cells = historybuf_cpu_cells(self->historybuf, y); + for (index_type x = 0; x < self->historybuf->xnum; x++) process_cell(pool, map, clone, cells + x); + } + } + LineBuf *second = self->linebuf, *first = second == self->main_linebuf ? self->alt_linebuf : self->main_linebuf; + for (index_type i = 0; i < self->lines * self->columns; i++) process_cell(pool, map, clone, first->cpu_cell_buf + i); + for (index_type i = 0; i < self->lines * self->columns; i++) process_cell(pool, map, clone, second->cpu_cell_buf + i); +} void screen_garbage_collect_hyperlink_pool(Screen *screen) { HyperLinkPool *pool = (HyperLinkPool*)screen->hyperlink_pool; pool->num_of_adds_since_garbage_collection = 0; if (!pool->max_link_id) return; - hyperlink_id_type *map = calloc(HYPERLINK_MAX_NUMBER + 4, sizeof(hyperlink_id_type)); - if (!map) fatal("Out of memory"); - hyperlink_id_type num = remap_hyperlink_ids(screen, map); - if (num) { - HyperLinkEntry *s, *tmp; - pool->max_link_id = 0; - HASH_ITER(hh, pool->hyperlinks, s, tmp) { - if (map[s->id]) { - s->id = map[s->id]; - pool->max_link_id = MAX(pool->max_link_id, s->id); - } else { - HASH_DEL(pool->hyperlinks, s); - free_hyperlink_entry(s); s = NULL; - } - } - } else clear_pool(pool); - free(map); + pool->max_link_id = 0; + id_id_map map = {0}; + vt_init(&map); + hyperlink_id_map clone = {0}; + if (!vt_init_clone(&clone, &pool->idmap)) fatal("Out of memory"); + vt_cleanup(&pool->map); vt_cleanup(&pool->idmap); + remap_hyperlink_ids(screen, &map, &clone); + for (hyperlink_id_map_itr i = vt_first(&clone); !vt_is_end(i); i = vt_next(i)) free((char*)i.data->val); + vt_clear(&map); vt_clear(&clone); } @@ -103,57 +133,40 @@ get_id_for_hyperlink(Screen *screen, const char *id, const char *url) { if (keylen < 0) keylen = strlen(key); else keylen = MIN(keylen, MAX_KEY_LEN - 2); // snprintf returns how many chars it would have written in case of truncation key[keylen] = 0; - HyperLinkEntry *s = NULL; - if (pool->hyperlinks) { - HASH_FIND_STR(pool->hyperlinks, key, s); - if (s) { - // Remove and re-add s so that it is the last entry in the hash table and - // The first entry is discarded when hash table is full. - HASH_DEL(pool->hyperlinks, s); - HASH_ADD_KEYPTR(hh, pool->hyperlinks, s->key, strlen(s->key), s); - return s->id; + hyperlink_map_itr itr = vt_get(&pool->map, key); + if (!vt_is_end(itr)) return itr.data->val; + if (pool->max_link_id >= HYPERLINK_MAX_NUMBER) { + if (pool->num_of_adds_since_garbage_collection > 128) screen_garbage_collect_hyperlink_pool(screen); + if (pool->max_link_id >= HYPERLINK_MAX_NUMBER) { + log_error("Too many hyperlinks, discarding hyperlink: %s", key); + return 0; } } - hyperlink_id_type new_id = 0; - if (pool->num_of_adds_since_garbage_collection >= MAX_ADDS_BEFORE_GC) screen_garbage_collect_hyperlink_pool(screen); - if (pool->max_link_id >= HYPERLINK_MAX_NUMBER && pool->hyperlinks) { - log_error("Too many hyperlinks, discarding oldest, this means some hyperlinks might be incorrect"); - new_id = pool->hyperlinks->id; - HyperLinkEntry *s = pool->hyperlinks; - HASH_DEL(pool->hyperlinks, s); - free_hyperlink_entry(s); s = NULL; - } - s = malloc(sizeof(HyperLinkEntry)); - if (!s) fatal("Out of memory"); - s->key = malloc(keylen + 1); - if (!s->key) fatal("Out of memory"); - memcpy((void*)s->key, key, keylen + 1); - s->id = new_id ? new_id : ++pool->max_link_id; - HASH_ADD_KEYPTR(hh, pool->hyperlinks, s->key, keylen, s); + hyperlink_id_type new_id = ++pool->max_link_id; + const char *skey = dupstr(key, keylen); + if (vt_is_end(vt_insert(&pool->map, skey, new_id))) fatal("Out of memory"); + if (vt_is_end(vt_insert(&pool->idmap, new_id, skey))) fatal("Out of memory"); pool->num_of_adds_since_garbage_collection++; - return s->id; + return new_id; } const char* get_hyperlink_for_id(const HYPERLINK_POOL_HANDLE handle, hyperlink_id_type id, bool only_url) { - const HyperLinkPool *pool = (HyperLinkPool*)handle; - HyperLinkEntry *s, *tmp; - HASH_ITER(hh, pool->hyperlinks, s, tmp) { - if (s->id == id) return only_url ? strstr(s->key, ":") + 1 : s->key; - } - return NULL; + HyperLinkPool *pool = (HyperLinkPool*)handle; + hyperlink_id_map_itr itr = vt_get(&pool->idmap, id); + if (vt_is_end(itr)) return NULL; + return only_url ? strstr(itr.data->val, ":") + 1 : itr.data->val; } - PyObject* -screen_hyperlinks_as_list(Screen *screen) { +screen_hyperlinks_as_set(Screen *screen) { HyperLinkPool *pool = (HyperLinkPool*)screen->hyperlink_pool; - PyObject *ans = PyList_New(0); - HyperLinkEntry *s, *tmp; - HASH_ITER(hh, pool->hyperlinks, s, tmp) { - PyObject *e = Py_BuildValue("sH", s->key, s->id); - PyList_Append(ans, e); - Py_DECREF(e); + RAII_PyObject(ans, PySet_New(0)); + if (ans) { + for (hyperlink_map_itr itr = vt_first(&pool->map); !vt_is_end(itr); itr = vt_next(itr)) { + RAII_PyObject(e, Py_BuildValue("sH", itr.data->key, itr.data->val)); + if (!e || PySet_Add(ans, e) != 0) return NULL; + } } - return ans; + Py_XINCREF(ans); return ans; } diff --git a/kitty/hyperlink.h b/kitty/hyperlink.h index 67e22381c..824c1d8ae 100644 --- a/kitty/hyperlink.h +++ b/kitty/hyperlink.h @@ -12,6 +12,5 @@ HYPERLINK_POOL_HANDLE alloc_hyperlink_pool(void); void free_hyperlink_pool(HYPERLINK_POOL_HANDLE); void clear_hyperlink_pool(HYPERLINK_POOL_HANDLE); hyperlink_id_type get_id_for_hyperlink(Screen*, const char*, const char*); -hyperlink_id_type remap_hyperlink_ids(Screen *self, hyperlink_id_type *map); -PyObject* screen_hyperlinks_as_list(Screen *screen); +PyObject* screen_hyperlinks_as_set(Screen *screen); void screen_garbage_collect_hyperlink_pool(Screen *screen); diff --git a/kitty/screen.c b/kitty/screen.c index 8fc26a4a6..ff9054bf8 100644 --- a/kitty/screen.c +++ b/kitty/screen.c @@ -606,30 +606,6 @@ set_active_hyperlink(Screen *self, char *id, char *url) { } } -hyperlink_id_type -remap_hyperlink_ids(Screen *self, hyperlink_id_type *map) { -#define PROCESS_CELL(cell) { hid = (cell).hyperlink_id; if (hid) { if (!map[hid]) map[hid] = ++num; (cell).hyperlink_id = map[hid]; }} - hyperlink_id_type num = 0, hid; - if (self->historybuf->count) { - for (index_type y = self->historybuf->count; y-- > 0;) { - CPUCell *cells = historybuf_cpu_cells(self->historybuf, y); - for (index_type x = 0; x < self->historybuf->xnum; x++) { - PROCESS_CELL(cells[x]); - } - } - } - LineBuf *second = self->linebuf, *first = second == self->main_linebuf ? self->alt_linebuf : self->main_linebuf; - for (index_type i = 0; i < self->lines * self->columns; i++) { - PROCESS_CELL(first->cpu_cell_buf[i]); - } - for (index_type i = 0; i < self->lines * self->columns; i++) { - PROCESS_CELL(second->cpu_cell_buf[i]); - } - return num; -#undef PROCESS_CELL -} - - static bool is_flag_pair(char_type a, char_type b) { return is_flag_codepoint(a) && is_flag_codepoint(b); } @@ -3467,8 +3443,8 @@ has_selection(Screen *self, PyObject *a UNUSED) { } static PyObject* -hyperlinks_as_list(Screen *self, PyObject *args UNUSED) { - return screen_hyperlinks_as_list(self); +hyperlinks_as_set(Screen *self, PyObject *args UNUSED) { + return screen_hyperlinks_as_set(self); } static PyObject* @@ -4802,7 +4778,7 @@ static PyMethodDef methods[] = { MND(erase_in_display, METH_VARARGS) MND(clear_scrollback, METH_NOARGS) MND(scroll_until_cursor_prompt, METH_VARARGS) - MND(hyperlinks_as_list, METH_NOARGS) + MND(hyperlinks_as_set, METH_NOARGS) MND(garbage_collect_hyperlink_pool, METH_NOARGS) MND(hyperlink_for_id, METH_O) MND(reverse_scroll, METH_VARARGS) diff --git a/kitty_tests/screen.py b/kitty_tests/screen.py index 754cf972b..1de42fef1 100644 --- a/kitty_tests/screen.py +++ b/kitty_tests/screen.py @@ -825,32 +825,32 @@ class TestScreen(BaseTest): s.draw('hij') self.ae(s.line(1).hyperlink_ids(), (1, 1, 2, 2, 2)) set_link() - self.ae([('a:url-a', 1), (':url-b', 2)], s.hyperlinks_as_list()) + self.ae({('a:url-a', 1), (':url-b', 2)}, s.hyperlinks_as_set()) s.garbage_collect_hyperlink_pool() - self.ae([('a:url-a', 1), (':url-b', 2)], s.hyperlinks_as_list()) + self.ae({('a:url-a', 1), (':url-b', 2)}, s.hyperlinks_as_set()) for i in range(s.lines + 2): s.linefeed() s.garbage_collect_hyperlink_pool() - self.ae([('a:url-a', 1), (':url-b', 2)], s.hyperlinks_as_list()) + self.ae({('a:url-a', 1), (':url-b', 2)}, s.hyperlinks_as_set()) for i in range(s.lines * 2): s.linefeed() s.garbage_collect_hyperlink_pool() - self.assertFalse(s.hyperlinks_as_list()) + self.assertFalse(s.hyperlinks_as_set()) set_link('url-a', 'x') s.draw('a') set_link('url-a', 'y') s.draw('a') set_link() - self.ae([('x:url-a', 1), ('y:url-a', 2)], s.hyperlinks_as_list()) + self.ae({('x:url-a', 1), ('y:url-a', 2)}, s.hyperlinks_as_set()) s = self.create_screen() set_link('u' * 2048) s.draw('a') - self.ae([(':' + 'u' * 2045, 1)], s.hyperlinks_as_list()) + self.ae({(':' + 'u' * 2045, 1)}, s.hyperlinks_as_set()) s = self.create_screen() set_link('u' * 2048, 'i' * 300) s.draw('a') - self.ae([('i'*256 + ':' + 'u' * (2045 - 256), 1)], s.hyperlinks_as_list()) + self.ae({('i'*256 + ':' + 'u' * (2045 - 256), 1)}, s.hyperlinks_as_set()) s = self.create_screen() set_link('1'), s.draw('1') @@ -859,19 +859,19 @@ class TestScreen(BaseTest): s.cursor.x = 1 set_link(), s.draw('X') self.ae(s.line(0).hyperlink_ids(), (1, 0, 3, 0, 0)) - self.ae([(':1', 1), (':2', 2), (':3', 3)], s.hyperlinks_as_list()) + self.ae({(':1', 1), (':2', 2), (':3', 3)}, s.hyperlinks_as_set()) s.garbage_collect_hyperlink_pool() - self.ae([(':1', 1), (':3', 2)], s.hyperlinks_as_list()) + self.ae({(':1', 1), (':3', 2)}, s.hyperlinks_as_set()) set_link('3'), s.draw('3') - self.ae([(':1', 1), (':3', 2)], s.hyperlinks_as_list()) + self.ae({(':1', 1), (':3', 2)}, s.hyperlinks_as_set()) set_link('4'), s.draw('4') - self.ae([(':1', 1), (':3', 2), (':4', 3)], s.hyperlinks_as_list()) + self.ae({(':1', 1), (':3', 2), (':4', 3)}, s.hyperlinks_as_set()) s = self.create_screen() set_link('1'), s.draw('1') set_link('2'), s.draw('2') set_link('1'), s.draw('1') - self.ae([(':2', 2), (':1', 1)], s.hyperlinks_as_list()) + self.ae({(':2', 2), (':1', 1)}, s.hyperlinks_as_set()) s = self.create_screen() set_link('1'), s.draw('12'), set_link(), s.draw('X'), set_link('1'), s.draw('3')