Merge pull request #32863 from vespa-engine/toregge/trim-down-posting…

…-list-cache-key

Trim down posting list cache key, use context to pass backing store f…

searchlib/src/tests/diskindex/posting_list_cache/posting_list_cache_test.cpp

@@ -15,7 +15,7 @@ class MockFile : public PostingListCache::IPostingListFileBacking {
 public:
     MockFile();
     ~MockFile() override;
-    PostingListHandle read(const PostingListCache::Key& key) const override;
+    PostingListHandle read(const PostingListCache::Key& key, PostingListCache::Context& ctx) const override;
     std::shared_ptr<BitVector> read(const PostingListCache::BitVectorKey& key, PostingListCache::Context& ctx) const override;
 };
 
@@ -27,9 +27,10 @@ MockFile::MockFile()
 MockFile::~MockFile() = default;
 
 PostingListHandle
-MockFile::read(const PostingListCache::Key& key) const
+MockFile::read(const PostingListCache::Key& key, PostingListCache::Context& ctx) const
 {
     EXPECT_NE(0, key.bit_length);
+    ctx.cache_miss = true;
     PostingListHandle handle;
     handle._allocSize = key.bit_length / 8;
     return handle;
@@ -57,8 +58,14 @@ class PostingListCacheTest : public ::testing::Test
     PostingListCache::Context _ctx;
     PostingListCacheTest();
     ~PostingListCacheTest() override;
-    PostingListHandle read() const { return _cache.read(_key); }
-    std::shared_ptr<BitVector> read_bv() { return _cache.read(_bv_key, _ctx); }
+    PostingListHandle read() {
+        _ctx.cache_miss = false;
+        return _cache.read(_key, _ctx);
+    }
+    std::shared_ptr<BitVector> read_bv() {
+        _ctx.cache_miss = false;
+        return _cache.read(_bv_key, _ctx);
+    }
 };
 
 PostingListCacheTest::PostingListCacheTest()
@@ -69,7 +76,6 @@ PostingListCacheTest::PostingListCacheTest()
       _bv_key(),
       _ctx(&_mock_file)
 {
-    _key.backing_store_file = &_mock_file;
 }
 
 PostingListCacheTest::~PostingListCacheTest() = default;
@@ -78,8 +84,11 @@ TEST_F(PostingListCacheTest, repeated_lookups_gives_hit)
 {
     _key.bit_length = 24 * 8;
     auto handle = read();
+    EXPECT_TRUE(_ctx.cache_miss);
     auto handle2 = read();
+    EXPECT_FALSE(_ctx.cache_miss);
     auto handle3 = read();
+    EXPECT_FALSE(_ctx.cache_miss);
     EXPECT_EQ(24, handle._allocSize);
     auto stats = _cache.get_stats();
     EXPECT_EQ(1, stats.misses);
@@ -125,10 +134,8 @@ TEST_F(PostingListCacheTest, repeated_bitvector_lookup_gives_hit)
 {
     _bv_key.lookup_result.idx = 1;
     _bv_key.file_id = 2;
-    _ctx.cache_miss = false;
     auto bv = read_bv();
     EXPECT_TRUE(_ctx.cache_miss);
-    _ctx.cache_miss = false;
     auto bv2 = read_bv();
     EXPECT_FALSE(_ctx.cache_miss);
     EXPECT_EQ(bv, bv2);

searchlib/src/vespa/searchlib/diskindex/field_index.cpp

@@ -182,12 +182,12 @@ FieldIndex::read_uncached_posting_list(const DictionaryLookupResult& lookup_resu
 }
 
 PostingListHandle
-FieldIndex::read(const IPostingListCache::Key& key) const
+FieldIndex::read(const IPostingListCache::Key& key, IPostingListCache::Context& ctx) const
 {
+    ctx.cache_miss = true;
     DictionaryLookupResult lookup_result;
     lookup_result.bitOffset = key.bit_offset;
     lookup_result.counts._bitLength = key.bit_length;
-    key.backing_store_file = nullptr; // Signal cache miss back to layer above cache
     return read_uncached_posting_list(lookup_result);
 }
 
@@ -202,13 +202,12 @@ FieldIndex::read_posting_list(const DictionaryLookupResult& lookup_result) const
         return read_uncached_posting_list(lookup_result);
     }
     IPostingListCache::Key key;
-    key.backing_store_file = this;
     key.file_id = _file_id;
     key.bit_offset = lookup_result.bitOffset;
     key.bit_length = lookup_result.counts._bitLength;
-    auto result = _posting_list_cache->read(key);
-    auto cache_hit = key.backing_store_file == this;
-    if (cache_hit && result._read_bytes != 0) {
+    IPostingListCache::Context ctx(this);
+    auto result = _posting_list_cache->read(key, ctx);
+    if (!ctx.cache_miss && result._read_bytes != 0) {
         _cache_disk_io_stats->add_cached_read_operation(result._read_bytes);
     }
     return result;

searchlib/src/vespa/searchlib/diskindex/field_index.h

@@ -73,7 +73,7 @@ class FieldIndex : public IPostingListCache::IPostingListFileBacking {
     bool open(const std::string& field_dir, const TuneFileSearch &tune_file_search);
     void reuse_files(const FieldIndex& rhs);
     index::PostingListHandle read_uncached_posting_list(const search::index::DictionaryLookupResult& lookup_result) const;
-    index::PostingListHandle read(const IPostingListCache::Key& key) const override;
+    index::PostingListHandle read(const IPostingListCache::Key& key, IPostingListCache::Context& ctx) const override;
     index::PostingListHandle read_posting_list(const search::index::DictionaryLookupResult& lookup_result) const;
     index::BitVectorDictionaryLookupResult lookup_bit_vector(const search::index::DictionaryLookupResult& lookup_result) const;
     std::shared_ptr<BitVector> read_uncached_bit_vector(index::BitVectorDictionaryLookupResult lookup_result) const;

searchlib/src/vespa/searchlib/diskindex/i_posting_list_cache.h

@@ -18,11 +18,10 @@ class IPostingListCache {
 public:
     class IPostingListFileBacking;
     struct Key {
-       mutable const IPostingListFileBacking* backing_store_file; // Used by backing store on cache miss
        uint64_t file_id;
        uint64_t bit_offset;
        uint64_t bit_length;
-       Key() noexcept : backing_store_file(nullptr), file_id(0), bit_offset(0), bit_length(0) { }
+       Key() noexcept : file_id(0), bit_offset(0), bit_length(0) { }
        size_t hash() const noexcept { return std::rotl(file_id, 40) + bit_offset; }
        bool operator==(const Key& rhs) const noexcept {
            // Don't check backing_store_file, it is just passed in key for convenience
@@ -56,11 +55,11 @@ class IPostingListCache {
     class IPostingListFileBacking {
     public:
         virtual ~IPostingListFileBacking() = default;
-        virtual search::index::PostingListHandle read(const Key& key) const = 0;
+        virtual search::index::PostingListHandle read(const Key& key, Context& ctx) const = 0;
         virtual std::shared_ptr<BitVector> read(const BitVectorKey& key, Context& ctx) const = 0;
     };
     virtual ~IPostingListCache() = default;
-    virtual search::index::PostingListHandle read(const Key& key) const = 0;
+    virtual search::index::PostingListHandle read(const Key& key, Context& ctx) const = 0;
     virtual std::shared_ptr<BitVector> read(const BitVectorKey& key, Context& ctx) const = 0;
     virtual vespalib::CacheStats get_stats() const = 0;
     virtual vespalib::CacheStats get_bitvector_stats() const = 0;

searchlib/src/vespa/searchlib/diskindex/posting_list_cache.cpp

@@ -19,18 +19,18 @@ class PostingListCache::BackingStore
 public:
     BackingStore();
     ~BackingStore();
-    bool read(const Key& key, PostingListHandle& value) const;
+    bool read(const Key& key, PostingListHandle& value, Context& ctx) const;
     bool read(const BitVectorKey& key, std::shared_ptr<BitVector>& value, Context& ctx) const;
 };
 
 PostingListCache::BackingStore::BackingStore() = default;
 PostingListCache::BackingStore::~BackingStore() = default;
 
 bool
-PostingListCache::BackingStore::read(const Key& key, PostingListHandle& value) const
+PostingListCache::BackingStore::read(const Key& key, PostingListHandle& value, Context& ctx) const
 {
     // TODO: Store a smaller copy if posting list is small
-    value = key.backing_store_file->read(key);
+    value = ctx.backing_store_file->read(key, ctx);
     return true;
 }
 
@@ -104,9 +104,9 @@ PostingListCache::PostingListCache(size_t max_bytes, size_t bitvector_max_bytes)
 PostingListCache::~PostingListCache() = default;
 
 PostingListHandle
-PostingListCache::read(const Key& key) const
+PostingListCache::read(const Key& key, Context& ctx) const
 {
-    return _cache->read(key);
+    return _cache->read(key, ctx);
 }
 
 std::shared_ptr<BitVector>

searchlib/src/vespa/searchlib/diskindex/posting_list_cache.h

@@ -22,7 +22,7 @@ class PostingListCache : public IPostingListCache {
 public:
     PostingListCache(size_t max_bytes, size_t bitvector_max_bytes);
     ~PostingListCache() override;
-    search::index::PostingListHandle read(const Key& key) const override;
+    search::index::PostingListHandle read(const Key& key, Context& ctx) const override;
     std::shared_ptr<BitVector> read(const BitVectorKey& key, Context& ctx) const override;
     vespalib::CacheStats get_stats() const override;
     vespalib::CacheStats get_bitvector_stats() const override;