Merge pull request #7042 from vcaputo/iteratedcache

RFC: Optionally cache hashmap iterated results

src/basic/hashmap.c

@@ -229,6 +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 last iterated_cache_get() */
+        bool cached:1;               /* whether this hashmap is being cached */
         HASHMAP_DEBUG_FIELDS         /* optional hashmap_debug_info */
 };
 
@@ -248,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 */
@@ -351,6 +364,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 +586,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)
 
@@ -737,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;
@@ -897,6 +935,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 +1081,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 +1319,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 +1343,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;
 }
 
@@ -1851,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;
+}

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)

src/journal/journal-internal.h

@@ -89,6 +89,7 @@ struct sd_journal {
         char *prefix;
 
         OrderedHashmap *files;
+        IteratedCache *files_cache;
         MMapCache *mmap;
 
         Location current_location;

src/journal/sd-journal.c

@@ -820,15 +820,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);
@@ -1737,9 +1743,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;
@@ -1985,6 +1995,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);

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();
 }