From 84dcca75b4a2c2274e220a1131ab9104c739d356 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sun, 8 Oct 2017 16:28:04 -0700 Subject: [PATCH 1/4] basic: track dirty state in HashmapBase This only adds marking the HashmapBase as dirty, no clearing of the dirty state happens yet. No functional changes. --- src/basic/hashmap.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index cc4423b2af..2e46b8bdeb 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -229,6 +229,7 @@ struct HashmapBase { unsigned n_direct_entries:3; /* Number of entries in direct storage. * Only valid if !has_indirect. */ bool from_pool:1; /* whether was allocated from mempool */ + bool dirty:1; /* whether dirtied since cache sync */ HASHMAP_DEBUG_FIELDS /* optional hashmap_debug_info */ }; @@ -351,6 +352,11 @@ static unsigned base_bucket_hash(HashmapBase *h, const void *p) { } #define bucket_hash(h, p) base_bucket_hash(HASHMAP_BASE(h), p) +static inline void base_set_dirty(HashmapBase *h) { + h->dirty = true; +} +#define hashmap_set_dirty(h) base_set_dirty(HASHMAP_BASE(h)) + static void get_hash_key(uint8_t hash_key[HASH_KEY_SIZE], bool reuse_is_ok) { static uint8_t current[HASH_KEY_SIZE]; static bool current_initialized = false; @@ -568,6 +574,7 @@ static void base_remove_entry(HashmapBase *h, unsigned idx) { bucket_mark_free(h, prev); n_entries_dec(h); + base_set_dirty(h); } #define remove_entry(h, idx) base_remove_entry(HASHMAP_BASE(h), idx) @@ -897,6 +904,8 @@ void internal_hashmap_clear(HashmapBase *h) { OrderedHashmap *lh = (OrderedHashmap*) h; lh->iterate_list_head = lh->iterate_list_tail = IDX_NIL; } + + base_set_dirty(h); } void internal_hashmap_clear_free(HashmapBase *h) { @@ -1041,6 +1050,8 @@ static int hashmap_base_put_boldly(HashmapBase *h, unsigned idx, h->debug.max_entries = MAX(h->debug.max_entries, n_entries(h)); #endif + base_set_dirty(h); + return 1; } #define hashmap_put_boldly(h, idx, swap, may_resize) \ @@ -1277,6 +1288,8 @@ int hashmap_replace(Hashmap *h, const void *key, void *value) { #endif e->b.key = key; e->value = value; + hashmap_set_dirty(h); + return 0; } @@ -1299,6 +1312,8 @@ int hashmap_update(Hashmap *h, const void *key, void *value) { e = plain_bucket_at(h, idx); e->value = value; + hashmap_set_dirty(h); + return 0; } From 45ea84d8edf57261a8e179f8058db6ba707dcde7 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Fri, 26 Jan 2018 16:38:01 -0800 Subject: [PATCH 2/4] basic: implement the IteratedCache Adds the basics of the IteratedCache and constructor support for the Hashmap and OrderedHashmap types. iterated_cache_get() is responsible for synchronizing the cache with the associated Hashmap and making it available to the caller at the supplied result pointers. Since iterated_cache_get() may need to allocate memory, it may fail, so callers must check the return value. On success, pointer arrays containing pointers to the associated Hashmap's keys and values, in as-iterated order, are returned in res_keys and res_values, respectively. Either may be supplied as NULL to inhibit caching of the keys or values, respectively. Note that if the cached Hashmap hasn't changed since the previous call to iterated_cache_get(), and it's not a call activating caching of the values or keys, the cost is effectively zero as the resulting pointers will simply refer to the previously returned arrays as-is. A cleanup function has also been added, iterated_cache_free(). This only frees the IteratedCache container and related arrays. The associated Hashmap, its keys, and values are not affected. Also note that the associated Hashmap does not automatically free its associated IteratedCache when freed. One could, in theory, safely access the arrays returned by a successful iterated_cache_get() call after its associated Hashmap has been freed, including the referenced values and keys. Provided the iterated_cache_get() was performed prior to the hashmap free, and that the type of hashmap free performed didn't free keys and/or values as well. --- src/basic/hashmap.c | 125 +++++++++++++++++++++++++++++++++++++++++++- src/basic/hashmap.h | 17 ++++++ 2 files changed, 141 insertions(+), 1 deletion(-) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index 2e46b8bdeb..32be96455d 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -229,7 +229,8 @@ struct HashmapBase { unsigned n_direct_entries:3; /* Number of entries in direct storage. * Only valid if !has_indirect. */ bool from_pool:1; /* whether was allocated from mempool */ - bool dirty:1; /* whether dirtied since cache sync */ + bool dirty:1; /* whether dirtied since last iterated_cache_get() */ + bool cached:1; /* whether this hashmap is being cached */ HASHMAP_DEBUG_FIELDS /* optional hashmap_debug_info */ }; @@ -249,6 +250,17 @@ struct Set { struct HashmapBase b; }; +typedef struct CacheMem { + const void **ptr; + size_t n_populated, n_allocated; + bool active:1; +} CacheMem; + +struct IteratedCache { + HashmapBase *hashmap; + CacheMem keys, values; +}; + DEFINE_MEMPOOL(hashmap_pool, Hashmap, 8); DEFINE_MEMPOOL(ordered_hashmap_pool, OrderedHashmap, 8); /* No need for a separate Set pool */ @@ -744,6 +756,25 @@ bool set_iterate(Set *s, Iterator *i, void **value) { (idx != IDX_NIL); \ (idx) = hashmap_iterate_entry((h), &(i))) +IteratedCache *internal_hashmap_iterated_cache_new(HashmapBase *h) { + IteratedCache *cache; + + assert(h); + assert(!h->cached); + + if (h->cached) + return NULL; + + cache = new0(IteratedCache, 1); + if (!cache) + return NULL; + + cache->hashmap = h; + h->cached = true; + + return cache; +} + static void reset_direct_storage(HashmapBase *h) { const struct hashmap_type_info *hi = &hashmap_type_info[h->type]; void *p; @@ -1866,3 +1897,95 @@ int set_put_strsplit(Set *s, const char *v, const char *separators, ExtractFlags return r; } } + +/* expand the cachemem if needed, return true if newly (re)activated. */ +static int cachemem_maintain(CacheMem *mem, unsigned size) { + int r = false; + + assert(mem); + + if (!GREEDY_REALLOC(mem->ptr, mem->n_allocated, size)) { + if (size > 0) + return -ENOMEM; + } + + if (!mem->active) + mem->active = r = true; + + return r; +} + +int iterated_cache_get(IteratedCache *cache, const void ***res_keys, const void ***res_values, unsigned *res_n_entries) { + bool sync_keys = false, sync_values = false; + unsigned size; + int r; + + assert(cache); + assert(cache->hashmap); + + size = n_entries(cache->hashmap); + + if (res_keys) { + r = cachemem_maintain(&cache->keys, size); + if (r < 0) + return r; + + sync_keys = r; + } else + cache->keys.active = false; + + if (res_values) { + r = cachemem_maintain(&cache->values, size); + if (r < 0) + return r; + + sync_values = r; + } else + cache->values.active = false; + + if (cache->hashmap->dirty) { + if (cache->keys.active) + sync_keys = true; + if (cache->values.active) + sync_values = true; + + cache->hashmap->dirty = false; + } + + if (sync_keys || sync_values) { + unsigned i, idx; + Iterator iter; + + i = 0; + HASHMAP_FOREACH_IDX(idx, cache->hashmap, iter) { + struct hashmap_base_entry *e; + + e = bucket_at(cache->hashmap, idx); + + if (sync_keys) + cache->keys.ptr[i] = e->key; + if (sync_values) + cache->values.ptr[i] = entry_value(cache->hashmap, e); + i++; + } + } + + if (res_keys) + *res_keys = cache->keys.ptr; + if (res_values) + *res_values = cache->values.ptr; + if (res_n_entries) + *res_n_entries = size; + + return 0; +} + +IteratedCache *iterated_cache_free(IteratedCache *cache) { + if (cache) { + free(cache->keys.ptr); + free(cache->values.ptr); + free(cache); + } + + return NULL; +} diff --git a/src/basic/hashmap.h b/src/basic/hashmap.h index 0eb763944c..b674910397 100644 --- a/src/basic/hashmap.h +++ b/src/basic/hashmap.h @@ -53,6 +53,8 @@ typedef struct Hashmap Hashmap; /* Maps keys to values */ typedef struct OrderedHashmap OrderedHashmap; /* Like Hashmap, but also remembers entry insertion order */ typedef struct Set Set; /* Stores just keys */ +typedef struct IteratedCache IteratedCache; /* Caches the iterated order of one of the above */ + /* Ideally the Iterator would be an opaque struct, but it is instantiated * by hashmap users, so the definition has to be here. Do not use its fields * directly. */ @@ -126,6 +128,9 @@ static inline OrderedHashmap *ordered_hashmap_free_free_free(OrderedHashmap *h) return (void*)hashmap_free_free_free(PLAIN_HASHMAP(h)); } +IteratedCache *iterated_cache_free(IteratedCache *cache); +int iterated_cache_get(IteratedCache *cache, const void ***res_keys, const void ***res_values, unsigned *res_n_entries); + HashmapBase *internal_hashmap_copy(HashmapBase *h); static inline Hashmap *hashmap_copy(Hashmap *h) { return (Hashmap*) internal_hashmap_copy(HASHMAP_BASE(h)); @@ -139,6 +144,14 @@ int internal_ordered_hashmap_ensure_allocated(OrderedHashmap **h, const struct h #define hashmap_ensure_allocated(h, ops) internal_hashmap_ensure_allocated(h, ops HASHMAP_DEBUG_SRC_ARGS) #define ordered_hashmap_ensure_allocated(h, ops) internal_ordered_hashmap_ensure_allocated(h, ops HASHMAP_DEBUG_SRC_ARGS) +IteratedCache *internal_hashmap_iterated_cache_new(HashmapBase *h); +static inline IteratedCache *hashmap_iterated_cache_new(Hashmap *h) { + return (IteratedCache*) internal_hashmap_iterated_cache_new(HASHMAP_BASE(h)); +} +static inline IteratedCache *ordered_hashmap_iterated_cache_new(OrderedHashmap *h) { + return (IteratedCache*) internal_hashmap_iterated_cache_new(HASHMAP_BASE(h)); +} + int hashmap_put(Hashmap *h, const void *key, void *value); static inline int ordered_hashmap_put(OrderedHashmap *h, const void *key, void *value) { return hashmap_put(PLAIN_HASHMAP(h), key, value); @@ -394,3 +407,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(OrderedHashmap*, ordered_hashmap_free_free_free); #define _cleanup_ordered_hashmap_free_ _cleanup_(ordered_hashmap_freep) #define _cleanup_ordered_hashmap_free_free_ _cleanup_(ordered_hashmap_free_freep) #define _cleanup_ordered_hashmap_free_free_free_ _cleanup_(ordered_hashmap_free_free_freep) + +DEFINE_TRIVIAL_CLEANUP_FUNC(IteratedCache*, iterated_cache_free); + +#define _cleanup_iterated_cache_free_ _cleanup_(iterated_cache_freep) From 5d4ba7f2b316a63ea5d98d749c5cb2cddf090854 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sun, 8 Oct 2017 16:52:56 -0700 Subject: [PATCH 3/4] journal: use IteratedCache in sd-journal This changes real_journal_next() to leverage the IteratedCache for accelerating iteration across the open journal files. journalctl timing comparisons with 100 journal files of 8MiB size party to this boot: Pre (~v235): # time ./journalctl -b --no-pager > /dev/null real 0m9.613s user 0m9.560s sys 0m0.053s # time ./journalctl -b --no-pager > /dev/null real 0m9.548s user 0m9.525s sys 0m0.023s # time ./journalctl -b --no-pager > /dev/null real 0m9.612s user 0m9.582s sys 0m0.030s Post-IteratedCache: # time ./journalctl -b --no-pager > /dev/null real 0m8.449s user 0m8.425s sys 0m0.024s # time ./journalctl -b --no-pager > /dev/null real 0m8.409s user 0m8.382s sys 0m0.027s # time ./journalctl -b --no-pager > /dev/null real 0m8.410s user 0m8.350s sys 0m0.061s ~12.5% improvement, the benefit increases the more log files there are. --- src/journal/journal-internal.h | 1 + src/journal/sd-journal.c | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/journal/journal-internal.h b/src/journal/journal-internal.h index 5ec87e83d7..d0d2842cc4 100644 --- a/src/journal/journal-internal.h +++ b/src/journal/journal-internal.h @@ -89,6 +89,7 @@ struct sd_journal { char *prefix; OrderedHashmap *files; + IteratedCache *files_cache; MMapCache *mmap; Location current_location; diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c index 8a1b161d8f..384a9b1bee 100644 --- a/src/journal/sd-journal.c +++ b/src/journal/sd-journal.c @@ -819,15 +819,21 @@ static int next_beyond_location(sd_journal *j, JournalFile *f, direction_t direc } static int real_journal_next(sd_journal *j, direction_t direction) { - JournalFile *f, *new_file = NULL; - Iterator i; + JournalFile *new_file = NULL; + unsigned i, n_files; + const void **files; Object *o; int r; assert_return(j, -EINVAL); assert_return(!journal_pid_changed(j), -ECHILD); - ORDERED_HASHMAP_FOREACH(f, j->files, i) { + r = iterated_cache_get(j->files_cache, NULL, &files, &n_files); + if (r < 0) + return r; + + for (i = 0; i < n_files; i++) { + JournalFile *f = (JournalFile *)files[i]; bool found; r = next_beyond_location(j, f, direction); @@ -1736,9 +1742,13 @@ static sd_journal *journal_new(int flags, const char *path) { } j->files = ordered_hashmap_new(&string_hash_ops); + if (!j->files) + goto fail; + + j->files_cache = ordered_hashmap_iterated_cache_new(j->files); j->directories_by_path = hashmap_new(&string_hash_ops); j->mmap = mmap_cache_new(); - if (!j->files || !j->directories_by_path || !j->mmap) + if (!j->files_cache || !j->directories_by_path || !j->mmap) goto fail; return j; @@ -1984,6 +1994,7 @@ _public_ void sd_journal_close(sd_journal *j) { sd_journal_flush_matches(j); ordered_hashmap_free_with_destructor(j->files, journal_file_close); + iterated_cache_free(j->files_cache); while ((d = hashmap_first(j->directories_by_path))) remove_directory(j, d); From 647c7b74407f6d9ea6ae4a87d0d223d56f5e5d72 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sat, 27 Jan 2018 13:10:39 -0800 Subject: [PATCH 4/4] test-hashmap: test IteratedCache Add some rudimentary testing of the new IteratedCache --- src/test/test-hashmap.c | 58 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/test/test-hashmap.c b/src/test/test-hashmap.c index dd9195425e..16ca27cd5f 100644 --- a/src/test/test-hashmap.c +++ b/src/test/test-hashmap.c @@ -80,6 +80,63 @@ static void test_string_compare_func(void) { assert_se(string_compare_func("fred", "fred") == 0); } +static void compare_cache(Hashmap *map, IteratedCache *cache) { + const void **keys = NULL, **values = NULL; + unsigned num, idx; + Iterator iter; + void *k, *v; + + assert_se(iterated_cache_get(cache, &keys, &values, &num) == 0); + assert_se(num == 0 || keys); + assert_se(num == 0 || values); + + idx = 0; + HASHMAP_FOREACH_KEY(v, k, map, iter) { + assert_se(v == values[idx]); + assert_se(k == keys[idx]); + + idx++; + } + + assert_se(idx == num); +} + +static void test_iterated_cache(void) { + Hashmap *m; + IteratedCache *c; + + assert_se(m = hashmap_new(NULL)); + assert_se(c = hashmap_iterated_cache_new(m)); + compare_cache(m, c); + + for (int stage = 0; stage < 100; stage++) { + + for (int i = 0; i < 100; i++) { + int foo = stage * 1000 + i; + + assert_se(hashmap_put(m, INT_TO_PTR(foo), INT_TO_PTR(foo + 777)) == 1); + } + + compare_cache(m, c); + + if (!(stage % 10)) { + for (int i = 0; i < 100; i++) { + int foo = stage * 1000 + i; + + assert_se(hashmap_remove(m, INT_TO_PTR(foo)) == INT_TO_PTR(foo + 777)); + } + + compare_cache(m, c); + } + } + + hashmap_clear(m); + compare_cache(m, c); + + assert_se(hashmap_free(m) == NULL); + assert_se(iterated_cache_free(c) == NULL); +} + int main(int argc, const char *argv[]) { test_hashmap_funcs(); test_ordered_hashmap_funcs(); @@ -89,4 +146,5 @@ int main(int argc, const char *argv[]) { test_uint64_compare_func(); test_trivial_compare_func(); test_string_compare_func(); + test_iterated_cache(); }