Replace fa_cache with htable cache (#160)

* Make htab cache generic
* Remove cache lookup on ldb_wal_index_node_retriever_mut
* Create MemoryContext inside cache_create

src/bench.h

@@ -18,4 +18,4 @@
 #else
 #define LanternBench(name, code) (code)
 #endif
-#endif  // LDB_BENCH_H
+#endif  // LDB_BENCH_H

src/hnsw/external_index.c

@@ -11,8 +11,8 @@
 #include <utils/hsearch.h>
 #include <utils/relcache.h>
 
-#include "cache.h"
 #include "extra_dirtied.h"
+#include "htab_cache.h"
 #include "insert.h"
 #include "options.h"
 #include "retriever.h"
@@ -512,12 +512,12 @@ static BlockNumber getBlockMapPageBlockNumber(uint32 *blockmap_page_group_index,
 
 BlockNumber getDataBlockNumber(RetrieverCtx *ctx, int id, bool add_to_extra_dirtied)
 {
-    Cache            *cache = &ctx->block_numbers_cache;
+    HTABCache        *cache = &ctx->block_numbers_cache;
     uint32           *blockmap_group_index = ctx->header_page_under_wal != NULL
                                                  ? ctx->header_page_under_wal->blockmap_page_group_index
                                                  : ctx->blockmap_page_group_index_cache;
     BlockNumber       blockmapno = getBlockMapPageBlockNumber(blockmap_group_index, id);
-    BlockNumber       blockno, blockno_from_cache;
+    BlockNumber       blockno;
     HnswBlockmapPage *blockmap_page;
     Page              page;
     Buffer            buf;
@@ -539,9 +539,9 @@ BlockNumber getDataBlockNumber(RetrieverCtx *ctx, int id, bool add_to_extra_dirt
     // clang-format on
 #endif
 
-    blockno_from_cache = cache_get_item(cache, &id);
-    if(blockno_from_cache != InvalidBlockNumber) {
-        return blockno_from_cache;
+    void *blockno_from_cache_p = cache_get_item(cache, &id);
+    if(blockno_from_cache_p != NULL) {
+        return *((BlockNumber *)blockno_from_cache_p);
     }
 
     // it is necessary to first check the extra dirtied pages for the blockmap page, in case we are in the
@@ -566,7 +566,7 @@ BlockNumber getDataBlockNumber(RetrieverCtx *ctx, int id, bool add_to_extra_dirt
 
     offset = id % HNSW_BLOCKMAP_BLOCKS_PER_PAGE;
     blockno = blockmap_page->blocknos[ offset ];
-    cache_set_item(cache, &id, blockmap_page->blocknos[ offset ]);
+    cache_set_item(cache, &id, &blockmap_page->blocknos[ offset ]);
     if(!idx_pagemap_prelocked) {
         UnlockReleaseBuffer(buf);
     }
@@ -583,7 +583,8 @@ void *ldb_wal_index_node_retriever(void *ctxp, int id)
     OffsetNumber    offset, max_offset;
     Buffer          buf = InvalidBuffer;
     bool            idx_page_prelocked = false;
-    void           *cached_node = fa_cache_get(&ctx->fa_cache, id);
+    void           *cached_node = cache_get_item(&ctx->node_cache, &id);
+
     if(cached_node != NULL) {
         return cached_node;
     }
@@ -628,7 +629,7 @@ void *ldb_wal_index_node_retriever(void *ctxp, int id)
                 LockBuffer(buf, BUFFER_LOCK_UNLOCK);
             }
 
-            fa_cache_insert(&ctx->fa_cache, id, nodepage->node);
+            cache_set_item(&ctx->node_cache, &id, nodepage->node);
 
             return nodepage->node;
 #endif
@@ -670,7 +671,7 @@ void *ldb_wal_index_node_retriever_mut(void *ctxp, int id)
     for(offset = FirstOffsetNumber; offset <= max_offset; offset = OffsetNumberNext(offset)) {
         nodepage = (HnswIndexTuple *)PageGetItem(page, PageGetItemId(page, offset));
         if(nodepage->id == (uint32)id) {
-            fa_cache_insert(&ctx->fa_cache, id, nodepage->node);
+            cache_set_item(&ctx->node_cache, &id, nodepage->node);
 
             return nodepage->node;
         }

src/hnsw/external_index.h

@@ -9,10 +9,9 @@
 #include <storage/bufmgr.h>  // Buffer
 #include <utils/relcache.h>  // Relation
 
-#include "cache.h"
 #include "extra_dirtied.h"
-#include "fa_cache.h"
 #include "hnsw.h"
+#include "htab_cache.h"
 #include "options.h"
 #include "usearch.h"
 
@@ -79,7 +78,7 @@ typedef struct
 
 typedef struct
 {
-    Cache block_numbers_cache;
+    HTABCache block_numbers_cache;
 
     Relation index_rel;
 
@@ -90,7 +89,7 @@ typedef struct
 
     ExtraDirtiedBufs *extra_dirted;
 
-    FullyAssociativeCache fa_cache;
+    HTABCache node_cache;
 
     dlist_head takenbuffers;
 } RetrieverCtx;

src/hnsw/htab_cache.c

@@ -0,0 +1,55 @@
+#include "htab_cache.h"
+
+#include "utils.h"
+
+HTABCache cache_create(const char *name)
+{
+    HTABCache cache;
+    HASHCTL   hctl;
+
+    hctl.keysize = sizeof(HTABCacheKey);
+    hctl.entrysize = sizeof(HTABCacheEntry);
+    hctl.hcxt = AllocSetContextCreate(CacheMemoryContext, "HTABCache", ALLOCSET_DEFAULT_SIZES);
+    HTAB *htab = hash_create(name, 1, &hctl, HASH_ELEM | HASH_CONTEXT | HASH_BLOBS);
+    cache.hctl = hctl;
+    cache.htab = htab;
+    return cache;
+}
+
+bool cache_remove(HTABCache *cache, HTABCacheKey *key)
+{
+    bool status;
+
+    hash_search(cache->htab, key, HASH_REMOVE, &status);
+
+    return status;
+}
+
+void *cache_get_item(HTABCache *cache, HTABCacheKey *key)
+{
+    bool status;
+
+    HTABCacheEntry *item = (HTABCacheEntry *)hash_search(cache->htab, key, HASH_FIND, &status);
+
+    if(!status) {
+        return NULL;
+    }
+
+    return item->value;
+}
+
+void cache_set_item(HTABCache *cache, HTABCacheKey *key, void *value)
+{
+    HTABCacheEntry *entry;
+
+    entry = (HTABCacheEntry *)hash_search(cache->htab, key, HASH_ENTER, NULL);
+    entry->value = value;
+}
+
+void cache_destroy(HTABCache *cache)
+{
+    MemoryContext old_context = MemoryContextSwitchTo(cache->hctl.hcxt);
+    hash_destroy(cache->htab);
+    MemoryContextSwitchTo(old_context);
+    MemoryContextDelete(cache->hctl.hcxt);
+}

src/hnsw/htab_cache.h

@@ -0,0 +1,42 @@
+#ifndef LDB_HNSW_HTAB_CACHE_H
+#define LDB_HNSW_HTAB_CACHE_H
+
+#include <postgres.h>
+
+#include <utils/hsearch.h>
+#include <utils/memutils.h>
+
+/*
+ * An abstract hash table (HTAB) cache that stores void pointers.
+ * Casting of pointers should be done externally when retrieving items.
+ *
+ * Note:
+ * - This cache stores void pointers, so type casting should be performed externally
+ *   when retrieving items from the cache.
+ * - The key and value pointers supplied for cache operations should have lifetimes
+ *   that outlive the cache itself.
+ * - If an item is not found in the cache, a NULL value will be returned.
+ * - A new memory context will be created in CacheMemoryContext when calling
+ *   cache_create, and it will be destroyed when calling cache_destroy.
+ */
+
+typedef int32 HTABCacheKey;
+typedef struct HTABCacheEntry
+{
+    HTABCacheKey key;
+    void        *value;
+} HTABCacheEntry;
+
+typedef struct HTABCache
+{
+    HTAB   *htab;
+    HASHCTL hctl;
+} HTABCache;
+
+HTABCache cache_create(const char *name);
+bool      cache_remove(HTABCache *cache, HTABCacheKey *key);
+void     *cache_get_item(HTABCache *cache, HTABCacheKey *key);
+void      cache_set_item(HTABCache *cache, HTABCacheKey *key, void *entry);
+void      cache_destroy(HTABCache *cache);
+
+#endif  // LDB_HNSW_HTAB_CACHE_H

src/hnsw/retriever.c

@@ -9,9 +9,8 @@
 #include <utils/hsearch.h>
 #include <utils/relcache.h>
 
-#include "cache.h"
 #include "external_index.h"
-#include "fa_cache.h"
+#include "htab_cache.h"
 #include "insert.h"
 
 RetrieverCtx *ldb_wal_retriever_area_init(Relation index_rel, HnswIndexHeaderPage *header_page_under_wal)
@@ -20,12 +19,13 @@ RetrieverCtx *ldb_wal_retriever_area_init(Relation index_rel, HnswIndexHeaderPag
     ctx->index_rel = index_rel;
     ctx->header_page_under_wal = header_page_under_wal;
     ctx->extra_dirted = extra_dirtied_new();
-    fa_cache_init(&ctx->fa_cache);
+
+    ctx->node_cache = cache_create("NodeCache");
 
     dlist_init(&ctx->takenbuffers);
 
     /* fill in a buffer with blockno index information, before spilling it to disk */
-    ctx->block_numbers_cache = cache_create();
+    ctx->block_numbers_cache = cache_create("BlockNumberCache");
 
     return ctx;
 }
@@ -55,6 +55,7 @@ void ldb_wal_retriever_area_reset(RetrieverCtx *ctx, HnswIndexHeaderPage *header
 void ldb_wal_retriever_area_fini(RetrieverCtx *ctx)
 {
     cache_destroy(&ctx->block_numbers_cache);
+    cache_destroy(&ctx->node_cache);
     dlist_mutable_iter miter;
     dlist_foreach_modify(miter, &ctx->takenbuffers)
     {