Feature - HNSW refactor - vector blocks - [MOD-5302] (#397)

* HNSW refactor 3 - [MOD-5302] (#389)

* moved vector_block

* make DataBlock available to use in vectors

* some general improvements

* implement data blocks in HNSW

* disabled serializer benchmarks and some tests

* more disable

* enabled serialization (for current implementation)

* added prefetch for range

* move generic meta of element to a separated vector

* added prefetch for metadata

* enabled save and load from bindings

* [TMP] return of the old HNSW
REVERT ME

* fix bm

* some changes in prefetch

* shortened BM

* reverted prefetches to by as before

* shorten BM

* small improvement for range

* packing structs

* unrelated performance improvement

* fix

* revert adding origin hnsw,
remove support for v1 and v2 serialization

* update for BM file

* some fixes and test updates

* [TEMP] disable 2 tests so coverage will run

* fix flow test

* more test fixes

* improved tests

* fix for bach iterator scan (needs a benchmark)

* for benchmark

* reverting some temporary changes

* more reverting

* fix for clang

* file name update

* another prefetch option

* few improvements

* some more trying

* final change

* another update to all but `hnsw.h`

* returned `increaseCapacity` responsibility to `addVector`

* make `hnsw.h` use blocks

* BF comment fix

* comment fix

* fix some tests

* fixed hnsw tests

* fixed hnsw-multi tests

* fixed almost all tiered HNSW tests

* Fix memory bookkeeping tests

* Fix memory bookkeeping tests 2

* fixed estimations and their tests

* review fixes

* fix review fix

* more review fixes

* move rounding up of initial capacity to a static function

* added comments on data blocks

* some optimizations (reduce the use of `getDataByInternalId`)

---------

Co-authored-by: alon <alonreshef24@gmail.com>

* HNSW blocks refactor - add lock to graph data struct (#390)

* Improved serializing code - [MOD-5372] (#391)

* improved serializing code

* review fixes

* HNSW Refactor - benchmarks - [MOD-5371] (#392)

* update benchmark files

* updated wget links

* publish serialization script

* another benchmark cleanup iteration

* review fixes

* Renaming "meta" variables (#394)

* renaming "meta" variables

* revert temp change

* Optimize Distance Functions - [MOD-5434] (#395)

* initial templated with masks implementations

* format

* tidy up

* enabled spaces tests back

* changed template type and handle residual first

* re-enabled benchmarks (keeping old names)

* download fix

* improved unit testing

* improved spaces benchmarks

* verify correctness

* some cleanup

* give up optimizing dim<16 for safety

* aligned serialization links

* added lots of comments

* added a test and small fix

* include opts only on x86 machines

* remove AVX512DQ references from the project (not in use)

* rename qty to dimension

* Update AVX_utils.h comments

* Optimize - implement align allocation for vector alignment - [MOD-5433] (#399)

* aligning query vector

* implement aligned allocation

* added alignment hing to VecSimIndexAbstract, used it in block allocation

* test fix

* review fixes

* set default value to the alignment hint (1 - any address is valid)

* refactor allocation header to have alignment flag, unify free function

* use alignment only on vector blocks

* changed default alignment value (0)

* updated tests

* added missing break

* improved comment

* removed alignment from allocator test

Author: GuyAv46

.gitignore

@@ -6,13 +6,14 @@
 /build/
 /dist/
 /venv/
-/deps/readies/
+/deps/
 /1/
 **/build/
 
 # Ignore benchmark fetched data but not the source file
 /tests/benchmark/data/*
 !/tests/benchmark/data/hnsw_indices
+!/tests/benchmark/data/serializer.py
 
 # Prerequisites
 *.d

README.md

@@ -27,7 +27,7 @@ All of the algorithms in this library is designed to work inside RediSearch and
 |-----------|--------|---------|-----------------|
 | FP32 Internal product |SSE, AVX, AVX512 | No SIMD support | No SIMD support |
 | FP32 L2 distance |SSE, AVX, AVX512| No SIMD support | No SIMD support |
-| FP64 Internal product |SSE, AVX, AVX512, AVX512DQ | No SIMD support | No SIMD support |
+| FP64 Internal product |SSE, AVX, AVX512 | No SIMD support | No SIMD support |
 | FP64 L2 distance |SSE, AVX, AVX512 | No SIMD support | No SIMD support |
 
 ### Flat (Brute Force)
@@ -92,5 +92,5 @@ tox -e flowenv
 
 # Benchmark
 
-To benchmark the capabilities of this library, follow the instructions in the [benchmarks user guide](docs/benchmarks.md).  
-If you'd like to create your own benchmarks, you can find more information in the [developer guide](docs/benchmarks_developer.md). 
+To benchmark the capabilities of this library, follow the instructions in the [benchmarks user guide](docs/benchmarks.md).
+If you'd like to create your own benchmarks, you can find more information in the [developer guide](docs/benchmarks_developer.md).

src/VecSim/CMakeLists.txt

@@ -19,14 +19,14 @@ add_library(VectorSimilarity ${VECSIM_LIBTYPE}
     index_factories/hnsw_factory.cpp
     index_factories/tiered_factory.cpp
     index_factories/index_factory.cpp
-    algorithms/brute_force/vector_block.cpp
     algorithms/hnsw/visited_nodes_handler.cpp
     vec_sim.cpp
     vec_sim_interface.cpp
     query_results.cpp
     info_iterator.cpp
     query_result_struct.cpp
     utils/vec_utils.cpp
+    utils/data_block.cpp
     memory/vecsim_malloc.cpp
     memory/vecsim_base.cpp
     ${HEADER_LIST}

src/VecSim/algorithms/brute_force/bfm_batch_iterator.h

@@ -24,7 +24,7 @@ class BFM_BatchIterator : public BF_BatchIterator<DataType, DistType> {
         this->scores.reserve(this->index_label_count);
         vecsim_stl::unordered_map<labelType, DistType> tmp_scores(this->index_label_count,
                                                                   this->allocator);
-        vecsim_stl::vector<VectorBlock *> blocks = this->index->getVectorBlocks();
+        auto &blocks = this->index->getVectorBlocks();
         VecSimQueryResult_Code rc;
 
         idType curr_id = 0;

src/VecSim/algorithms/brute_force/bfs_batch_iterator.h

@@ -22,7 +22,7 @@ class BFS_BatchIterator : public BF_BatchIterator<DataType, DistType> {
     inline VecSimQueryResult_Code calculateScores() override {
         this->index_label_count = this->index->indexLabelCount();
         this->scores.reserve(this->index_label_count);
-        vecsim_stl::vector<VectorBlock *> blocks = this->index->getVectorBlocks();
+        auto &blocks = this->index->getVectorBlocks();
         VecSimQueryResult_Code rc;
 
         idType curr_id = 0;

src/VecSim/algorithms/brute_force/brute_force.h

@@ -6,7 +6,7 @@
 
 #pragma once
 
-#include "vector_block.h"
+#include "VecSim/utils/data_block.h"
 #include "VecSim/vec_sim_index.h"
 #include "VecSim/spaces/spaces.h"
 #include "VecSim/utils/vecsim_stl.h"
@@ -29,20 +29,19 @@ template <typename DataType, typename DistType>
 class BruteForceIndex : public VecSimIndexAbstract<DistType> {
 protected:
     vecsim_stl::vector<labelType> idToLabelMapping;
-    vecsim_stl::vector<VectorBlock *> vectorBlocks;
+    vecsim_stl::vector<DataBlock> vectorBlocks;
     idType count;
 
 public:
     BruteForceIndex(const BFParams *params, const AbstractIndexInitParams &abstractInitParams);
 
     size_t indexSize() const override;
     size_t indexCapacity() const override;
-    void increaseCapacity() override;
-    vecsim_stl::vector<DistType> computeBlockScores(VectorBlock *block, const void *queryBlob,
+    vecsim_stl::vector<DistType> computeBlockScores(const DataBlock &block, const void *queryBlob,
                                                     void *timeoutCtx,
                                                     VecSimQueryResult_Code *rc) const;
     inline DataType *getDataByInternalId(idType id) const {
-        return (DataType *)vectorBlocks.at(id / this->blockSize)->getVector(id % this->blockSize);
+        return (DataType *)vectorBlocks.at(id / this->blockSize).getElement(id % this->blockSize);
     }
     virtual VecSimQueryResult_List topKQuery(const void *queryBlob, size_t k,
                                              VecSimQueryParams *queryParams) const override;
@@ -56,7 +55,7 @@ class BruteForceIndex : public VecSimIndexAbstract<DistType> {
     bool preferAdHocSearch(size_t subsetSize, size_t k, bool initial_check) const override;
     inline labelType getVectorLabel(idType id) const { return idToLabelMapping.at(id); }
 
-    inline vecsim_stl::vector<VectorBlock *> getVectorBlocks() const { return vectorBlocks; }
+    inline const vecsim_stl::vector<DataBlock> &getVectorBlocks() const { return vectorBlocks; }
     inline const labelType getLabelByInternalId(idType internal_id) const {
         return idToLabelMapping.at(internal_id);
     }
@@ -72,7 +71,7 @@ class BruteForceIndex : public VecSimIndexAbstract<DistType> {
     // without duplicates in tiered index). Caller should hold the flat buffer lock for read.
     virtual inline vecsim_stl::set<labelType> getLabelsSet() const = 0;
 
-    virtual ~BruteForceIndex();
+    virtual ~BruteForceIndex() = default;
 #ifdef BUILD_TESTS
     /**
      * @brief Used for testing - store vector(s) data associated with a given label. This function
@@ -93,7 +92,30 @@ class BruteForceIndex : public VecSimIndexAbstract<DistType> {
     // Private internal function that implements generic single vector deletion.
     virtual void removeVector(idType id);
 
-    inline VectorBlock *getVectorVectorBlock(idType id) const {
+    inline void growByBlock() {
+        assert(vectorBlocks.size() == 0 || vectorBlocks.back().getLength() == this->blockSize);
+        vectorBlocks.emplace_back(this->blockSize, this->dataSize, this->allocator,
+                                  this->alignment);
+        idToLabelMapping.resize(idToLabelMapping.size() + this->blockSize);
+        idToLabelMapping.shrink_to_fit();
+        resizeLabelLookup(idToLabelMapping.size());
+    }
+
+    inline void shrinkByBlock() {
+        assert(indexCapacity() > 0); // should not be called when index is empty
+
+        // remove last block (should be empty)
+        assert(vectorBlocks.size() > 0 && vectorBlocks.back().getLength() == 0);
+        vectorBlocks.pop_back();
+
+        // remove a block size of labels.
+        assert(idToLabelMapping.size() >= this->blockSize);
+        idToLabelMapping.resize(idToLabelMapping.size() - this->blockSize);
+        idToLabelMapping.shrink_to_fit();
+        resizeLabelLookup(idToLabelMapping.size());
+    }
+
+    inline DataBlock &getVectorVectorBlock(idType id) {
         return vectorBlocks.at(id / this->blockSize);
     }
     inline size_t getVectorRelativeIndex(idType id) const { return id % this->blockSize; }
@@ -111,6 +133,7 @@ class BruteForceIndex : public VecSimIndexAbstract<DistType> {
     // inline label to id setters that need to be implemented by derived class
     virtual inline void replaceIdOfLabel(labelType label, idType new_id, idType old_id) = 0;
     virtual inline void setVectorId(labelType label, idType id) = 0;
+    virtual inline void resizeLabelLookup(size_t new_max_elements) = 0;
 
     virtual inline VecSimBatchIterator *
     newBatchIterator_Instance(void *queryBlob, VecSimQueryParams *queryParams) const = 0;
@@ -129,41 +152,35 @@ BruteForceIndex<DataType, DistType>::BruteForceIndex(
     : VecSimIndexAbstract<DistType>(abstractInitParams), idToLabelMapping(this->allocator),
       vectorBlocks(this->allocator), count(0) {
     assert(VecSimType_sizeof(this->vecType) == sizeof(DataType));
-    this->idToLabelMapping.resize(params->initialCapacity);
-}
-
-template <typename DataType, typename DistType>
-BruteForceIndex<DataType, DistType>::~BruteForceIndex() {
-    for (auto &vectorBlock : this->vectorBlocks) {
-        delete vectorBlock;
-    }
+    // Round up the initial capacity to the nearest multiple of the block size.
+    size_t initialCapacity = RoundUpInitialCapacity(params->initialCapacity, this->blockSize);
+    this->idToLabelMapping.resize(initialCapacity);
+    this->vectorBlocks.reserve(initialCapacity / this->blockSize);
 }
 
 /******************** Implementation **************/
 
 template <typename DataType, typename DistType>
 void BruteForceIndex<DataType, DistType>::appendVector(const void *vector_data, labelType label) {
-    assert(indexCapacity() > indexSize());
     // Give the vector new id and increase count.
     idType id = this->count++;
 
-    // Get the last vectors block to store the vector in (we assume that it's not full yet).
-    VectorBlock *vectorBlock = this->vectorBlocks.back();
-    assert(vectorBlock == getVectorVectorBlock(id));
-
-    // add vector data to vectorBlock
-    vectorBlock->addVector(vector_data);
+    // Resize the index if needed.
+    if (indexSize() > indexCapacity()) {
+        growByBlock();
+    } else if (id % this->blockSize == 0) {
+        // If we we didn't reach the initial capacity but the last block is full, add a new block
+        // only.
+        this->vectorBlocks.emplace_back(this->blockSize, this->dataSize, this->allocator,
+                                        this->alignment);
+    }
 
-    // if idToLabelMapping is full,
-    // resize and align idToLabelMapping by blockSize
-    size_t idToLabelMapping_size = this->idToLabelMapping.size();
+    // Get the last vectors block to store the vector in.
+    DataBlock &vectorBlock = this->vectorBlocks.back();
+    assert(&vectorBlock == &getVectorVectorBlock(id));
 
-    if (id >= idToLabelMapping_size) {
-        size_t last_block_vectors_count = id % this->blockSize;
-        this->idToLabelMapping.resize(
-            idToLabelMapping_size + this->blockSize - last_block_vectors_count, 0);
-        this->idToLabelMapping.shrink_to_fit();
-    }
+    // add vector data to vectorBlock
+    vectorBlock.addElement(vector_data);
 
     // add label to idToLabelMapping
     setVectorLabel(id, label);
@@ -180,10 +197,10 @@ void BruteForceIndex<DataType, DistType>::removeVector(idType id_to_delete) {
     labelType last_idx_label = getVectorLabel(last_idx);
 
     // Get last vector data.
-    VectorBlock *last_vector_block = vectorBlocks.back();
-    assert(last_vector_block == getVectorVectorBlock(last_idx));
+    DataBlock &last_vector_block = vectorBlocks.back();
+    assert(&last_vector_block == &getVectorVectorBlock(last_idx));
 
-    void *last_vector_data = last_vector_block->removeAndFetchLastVector();
+    void *last_vector_data = last_vector_block.removeAndFetchLastElement();
 
     // If we are *not* trying to remove the last vector, update mapping and move
     // the data of the last vector in the index in place of the deleted vector.
@@ -198,27 +215,16 @@ void BruteForceIndex<DataType, DistType>::removeVector(idType id_to_delete) {
         replaceIdOfLabel(last_idx_label, id_to_delete, last_idx);
 
         // Get the vectorBlock and the relative index of the deleted id.
-        VectorBlock *deleted_vectorBlock = getVectorVectorBlock(id_to_delete);
+        DataBlock &deleted_vectorBlock = getVectorVectorBlock(id_to_delete);
         size_t id_to_delete_rel_idx = getVectorRelativeIndex(id_to_delete);
 
         // Put data of last vector inplace of the deleted vector.
-        deleted_vectorBlock->updateVector(id_to_delete_rel_idx, last_vector_data);
+        deleted_vectorBlock.updateElement(id_to_delete_rel_idx, last_vector_data);
     }
 
     // If the last vector block is emtpy.
-    if (last_vector_block->getLength() == 0) {
-        delete last_vector_block;
-        this->vectorBlocks.pop_back();
-
-        // Resize and align the idToLabelMapping.
-        size_t idToLabel_size = idToLabelMapping.size();
-        // If the new size is smaller by at least one block comparing to the idToLabelMapping
-        // align to be a multiplication of block size and resize by one block.
-        if (this->count + this->blockSize <= idToLabel_size) {
-            size_t vector_to_align_count = idToLabel_size % this->blockSize;
-            this->idToLabelMapping.resize(idToLabel_size - this->blockSize - vector_to_align_count);
-            this->idToLabelMapping.shrink_to_fit();
-        }
+    if (last_vector_block.getLength() == 0) {
+        shrinkByBlock();
     }
 }
 
@@ -229,29 +235,23 @@ size_t BruteForceIndex<DataType, DistType>::indexSize() const {
 
 template <typename DataType, typename DistType>
 size_t BruteForceIndex<DataType, DistType>::indexCapacity() const {
-    return this->blockSize * this->vectorBlocks.size();
-}
-
-template <typename DataType, typename DistType>
-void BruteForceIndex<DataType, DistType>::increaseCapacity() {
-    size_t vector_bytes_count = this->dim * VecSimType_sizeof(this->vecType);
-    auto *new_vector_block =
-        new (this->allocator) VectorBlock(this->blockSize, vector_bytes_count, this->allocator);
-    this->vectorBlocks.push_back(new_vector_block);
+    return this->idToLabelMapping.size();
 }
 
 // Compute the score for every vector in the block by using the given distance function.
 template <typename DataType, typename DistType>
-vecsim_stl::vector<DistType> BruteForceIndex<DataType, DistType>::computeBlockScores(
-    VectorBlock *block, const void *queryBlob, void *timeoutCtx, VecSimQueryResult_Code *rc) const {
-    size_t len = block->getLength();
+vecsim_stl::vector<DistType>
+BruteForceIndex<DataType, DistType>::computeBlockScores(const DataBlock &block,
+                                                        const void *queryBlob, void *timeoutCtx,
+                                                        VecSimQueryResult_Code *rc) const {
+    size_t len = block.getLength();
     vecsim_stl::vector<DistType> scores(len, this->allocator);
     for (size_t i = 0; i < len; i++) {
         if (VECSIM_TIMEOUT(timeoutCtx)) {
             *rc = VecSim_QueryResult_TimedOut;
             return scores;
         }
-        scores[i] = this->dist_func(block->getVector(i), queryBlob, this->dim);
+        scores[i] = this->distFunc(block.getElement(i), queryBlob, this->dim);
     }
     *rc = VecSim_QueryResult_OK;
     return scores;
@@ -264,7 +264,7 @@ BruteForceIndex<DataType, DistType>::topKQuery(const void *queryBlob, size_t k,
 
     VecSimQueryResult_List rl = {0};
     void *timeoutCtx = queryParams ? queryParams->timeoutCtx : NULL;
-    this->last_mode = STANDARD_KNN;
+    this->lastMode = STANDARD_KNN;
 
     if (0 == k) {
         rl.results = array_new<VecSimQueryResult>(0);
@@ -276,7 +276,7 @@ BruteForceIndex<DataType, DistType>::topKQuery(const void *queryBlob, size_t k,
         getNewMaxPriorityQueue();
     // For every block, compute its vectors scores and update the Top candidates max heap
     idType curr_id = 0;
-    for (auto vectorBlock : this->vectorBlocks) {
+    for (auto &vectorBlock : this->vectorBlocks) {
         auto scores = computeBlockScores(vectorBlock, queryBlob, timeoutCtx, &rl.code);
         if (VecSim_OK != rl.code) {
             delete TopCandidates;
@@ -314,7 +314,7 @@ BruteForceIndex<DataType, DistType>::rangeQuery(const void *queryBlob, double ra
                                                 VecSimQueryParams *queryParams) const {
     auto rl = (VecSimQueryResult_List){0};
     void *timeoutCtx = queryParams ? queryParams->timeoutCtx : nullptr;
-    this->last_mode = RANGE_QUERY;
+    this->lastMode = RANGE_QUERY;
 
     // Compute scores in every block and save results that are within the range.
     auto res_container =
@@ -323,7 +323,7 @@ BruteForceIndex<DataType, DistType>::rangeQuery(const void *queryBlob, double ra
     DistType radius_ = DistType(radius);
     idType curr_id = 0;
     rl.code = VecSim_QueryResult_OK;
-    for (auto vectorBlock : this->vectorBlocks) {
+    for (auto &vectorBlock : this->vectorBlocks) {
         auto scores = computeBlockScores(vectorBlock, queryBlob, timeoutCtx, &rl.code);
         if (VecSim_OK != rl.code) {
             break;
@@ -458,7 +458,7 @@ bool BruteForceIndex<DataType, DistType>::preferAdHocSearch(size_t subsetSize, s
         }
     }
     // Set the mode - if this isn't the initial check, we switched mode form batches to ad-hoc.
-    this->last_mode =
+    this->lastMode =
         res ? (initial_check ? HYBRID_ADHOC_BF : HYBRID_BATCHES_TO_ADHOC_BF) : HYBRID_BATCHES;
     return res;
 }

src/VecSim/algorithms/brute_force/brute_force_friend_tests.h

@@ -17,4 +17,5 @@ INDEX_TEST_FRIEND_CLASS(BruteForceTest_test_delete_swap_block_Test)
 INDEX_TEST_FRIEND_CLASS(BruteForceTest_test_dynamic_bf_info_iterator_Test)
 INDEX_TEST_FRIEND_CLASS(BruteForceTest_brute_force_zero_minimal_capacity_Test)
 INDEX_TEST_FRIEND_CLASS(BruteForceTest_preferAdHocOptimization_Test)
+INDEX_TEST_FRIEND_CLASS(IndexAllocatorTest_test_bf_index_block_size_1_Test)
 INDEX_TEST_FRIEND_CLASS(BM_VecSimBasics)

src/VecSim/algorithms/brute_force/brute_force_multi.h

@@ -56,6 +56,10 @@ class BruteForceIndex_Multi : public BruteForceIndex<DataType, DistType> {
 
     inline void replaceIdOfLabel(labelType label, idType new_id, idType old_id) override;
 
+    inline void resizeLabelLookup(size_t new_max_elements) override {
+        labelToIdsLookup.reserve(new_max_elements);
+    }
+
     inline bool isLabelExists(labelType label) override {
         return labelToIdsLookup.find(label) != labelToIdsLookup.end();
     }
@@ -198,7 +202,7 @@ double BruteForceIndex_Multi<DataType, DistType>::getDistanceFrom(labelType labe
 
     DistType dist = std::numeric_limits<DistType>::infinity();
     for (auto id : IDs->second) {
-        DistType d = this->dist_func(this->getDataByInternalId(id), vector_data, this->dim);
+        DistType d = this->distFunc(this->getDataByInternalId(id), vector_data, this->dim);
         dist = (dist < d) ? dist : d;
     }