Skip to content

Commit e91cb52

Browse files
committed
Fix incorrect assertion in BM_VecSimBasics::UpdateAtBlockSize benchmark (#819)
* VecSimIndexInterface: introduce indexMetaDataCapacity for testing returns metadata containers capacity * cover for tiered index (cherry picked from commit f24b65a)
1 parent dfd71f9 commit e91cb52

File tree

7 files changed

+62
-5
lines changed

7 files changed

+62
-5
lines changed

src/VecSim/algorithms/brute_force/brute_force.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ class BruteForceIndex : public VecSimIndexAbstract<DistType> {
9292

9393
return actual_stored_vec;
9494
}
95+
96+
size_t indexMetaDataCapacity() const override { return idToLabelMapping.capacity(); }
9597
#endif
9698

9799
protected:

src/VecSim/algorithms/hnsw/hnsw.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,8 @@ class HNSWIndex : public VecSimIndexAbstract<DistType>,
327327

328328
return actual_stored_vec;
329329
}
330+
331+
size_t indexMetaDataCapacity() const override { return idToMetaData.capacity(); }
330332
#endif
331333

332334
protected:

src/VecSim/algorithms/hnsw/hnsw_tiered.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,10 @@ class TieredHNSWIndex : public VecSimTieredIndex<DataType, DistType> {
226226
}
227227
#ifdef BUILD_TESTS
228228
void getDataByLabel(labelType label, std::vector<std::vector<DataType>> &vectors_output) const;
229+
size_t indexMetaDataCapacity() const override {
230+
return this->backendIndex->indexMetaDataCapacity() +
231+
this->frontendIndex->indexMetaDataCapacity();
232+
}
229233
#endif
230234
};
231235

src/VecSim/vec_sim_interface.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,4 +262,16 @@ struct VecSimIndexInterface : public VecsimBaseObject {
262262
inline static void setWriteMode(VecSimWriteMode mode) {
263263
VecSimIndexInterface::asyncWriteMode = mode;
264264
}
265+
#ifdef BUILD_TESTS
266+
/**
267+
* @brief get the capacity of the meta data containers.
268+
*
269+
* @return The capacity of the meta data containers in number of elements.
270+
* The value returned from this function may differ from the indexCapacity() function. For
271+
* example, in HNSW, the capacity of the meta data containers is the capacity of the labels
272+
* lookup table, while the capacity of the data containers is the capacity of the vectors
273+
* container.
274+
*/
275+
virtual size_t indexMetaDataCapacity() const = 0;
276+
#endif
265277
};

tests/benchmark/bm_vecsim_basics.h

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,9 @@ void BM_VecSimBasics<index_type_t>::UpdateAtBlockSize(benchmark::State &st) {
303303
// Calculate vectors needed to reach next block boundary
304304
size_t vecs_to_blocksize =
305305
BM_VecSimGeneral::block_size - (initial_index_size % BM_VecSimGeneral::block_size);
306+
size_t initial_index_cap = index->indexMetaDataCapacity();
307+
assert(initial_index_cap == N_VECTORS + vecs_to_blocksize);
308+
306309
assert(vecs_to_blocksize < BM_VecSimGeneral::block_size);
307310
labelType initial_label_count = index->indexLabelCount();
308311
labelType curr_label = initial_label_count;
@@ -327,24 +330,29 @@ void BM_VecSimBasics<index_type_t>::UpdateAtBlockSize(benchmark::State &st) {
327330

328331
// Benchmark loop: repeatedly delete/add same vector to trigger grow-shrink cycles
329332
labelType label_to_update = curr_label - 1;
330-
size_t index_cap = index->indexCapacity();
333+
size_t index_cap = index->indexMetaDataCapacity();
334+
std::cout << "index_cap after adding vectors " << index_cap << std::endl;
335+
assert(index_cap == initial_index_cap + BM_VecSimGeneral::block_size);
336+
331337
for (auto _ : st) {
332338
// Remove the vector directly from hnsw
333339
size_t ret = VecSimIndex_DeleteVector(
334340
INDICES[st.range(0) == VecSimAlgo_TIERED ? VecSimAlgo_HNSWLIB : st.range(0)],
335341
label_to_update);
336342
assert(ret == 1);
337-
assert(index->indexCapacity() == index_cap - BM_VecSimGeneral::block_size);
338-
// Capacity should shrink by one block after deletion
343+
344+
// Capacity should not change
345+
size_t curr_cap = index->indexMetaDataCapacity();
346+
assert(curr_cap == index_cap);
339347
ret = VecSimIndex_AddVector(index, QUERIES[(added_vec_count - 1) % N_QUERIES].data(),
340348
label_to_update);
341349
assert(ret == 1);
342350
BM_VecSimGeneral::mock_thread_pool.thread_pool_wait();
343351
assert(VecSimIndex_IndexSize(
344352
INDICES[st.range(0) == VecSimAlgo_TIERED ? VecSimAlgo_HNSWLIB : st.range(0)]) ==
345353
N_VECTORS + added_vec_count);
346-
// Capacity should grow back to original size after addition
347-
assert(index->indexCapacity() == index_cap);
354+
// Capacity should not change
355+
assert(index->indexMetaDataCapacity() == index_cap);
348356
}
349357
assert(VecSimIndex_IndexSize(index) == N_VECTORS + added_vec_count);
350358

tests/unit/test_allocator.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ TYPED_TEST(IndexAllocatorTest, test_bf_index_block_size_1) {
120120

121121
ASSERT_EQ(bfIndex->indexCapacity(), expected_map_containers_size);
122122
ASSERT_EQ(bfIndex->idToLabelMapping.capacity(), expected_map_containers_size);
123+
ASSERT_EQ(bfIndex->indexMetaDataCapacity(), expected_map_containers_size);
123124
ASSERT_EQ(bfIndex->idToLabelMapping.size(), expected_map_containers_size);
124125
ASSERT_GE(bfIndex->labelToIdLookup.bucket_count(), expected_map_containers_size);
125126
};
@@ -530,6 +531,7 @@ TYPED_TEST(IndexAllocatorTest, test_hnsw_reclaim_memory) {
530531
ASSERT_EQ(hnswIndex->getStoredVectorsCount(), expected_size);
531532

532533
ASSERT_EQ(hnswIndex->idToMetaData.capacity(), expected_map_containers_size);
534+
ASSERT_EQ(hnswIndex->indexMetaDataCapacity(), expected_map_containers_size);
533535
ASSERT_EQ(hnswIndex->idToMetaData.size(), expected_map_containers_size);
534536
ASSERT_GE(hnswIndex->labelLookup.bucket_count(), expected_map_containers_size);
535537
// Also validate that there are no unidirectional connections (these add memory to the

tests/unit/test_hnsw_tiered.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3795,6 +3795,11 @@ TYPED_TEST(HNSWTieredIndexTestBasic, HNSWResize) {
37953795
ASSERT_EQ(tiered_index->getMainIndexGuardWriteLockCount(), resize_operations);
37963796
ASSERT_EQ(hnsw_index->indexSize(), 1);
37973797
ASSERT_EQ(hnsw_index->indexCapacity(), blockSize);
3798+
ASSERT_EQ(hnsw_index->indexMetaDataCapacity(), blockSize);
3799+
ASSERT_EQ(tiered_index->frontendIndex->indexMetaDataCapacity(), 0);
3800+
ASSERT_EQ(tiered_index->indexMetaDataCapacity(),
3801+
hnsw_index->indexMetaDataCapacity() +
3802+
tiered_index->frontendIndex->indexMetaDataCapacity());
37983803

37993804
// add up to block size
38003805
for (size_t i = 1; i < blockSize; i++) {
@@ -3805,6 +3810,11 @@ TYPED_TEST(HNSWTieredIndexTestBasic, HNSWResize) {
38053810
ASSERT_EQ(tiered_index->getMainIndexGuardWriteLockCount(), resize_operations);
38063811
ASSERT_EQ(hnsw_index->indexSize(), blockSize);
38073812
ASSERT_EQ(hnsw_index->indexCapacity(), blockSize);
3813+
ASSERT_EQ(hnsw_index->indexMetaDataCapacity(), blockSize);
3814+
ASSERT_EQ(tiered_index->frontendIndex->indexMetaDataCapacity(), 0);
3815+
ASSERT_EQ(tiered_index->indexMetaDataCapacity(),
3816+
hnsw_index->indexMetaDataCapacity() +
3817+
tiered_index->frontendIndex->indexMetaDataCapacity());
38083818

38093819
// add one more vector to trigger another resize
38103820
GenerateAndAddVector<TEST_DATA_T>(tiered_index, dim, blockSize);
@@ -3814,6 +3824,11 @@ TYPED_TEST(HNSWTieredIndexTestBasic, HNSWResize) {
38143824
ASSERT_EQ(tiered_index->getMainIndexGuardWriteLockCount(), resize_operations);
38153825
ASSERT_EQ(hnsw_index->indexSize(), blockSize + 1);
38163826
ASSERT_EQ(hnsw_index->indexCapacity(), 2 * blockSize);
3827+
ASSERT_EQ(hnsw_index->indexMetaDataCapacity(), 2 * blockSize);
3828+
ASSERT_EQ(tiered_index->frontendIndex->indexMetaDataCapacity(), 0);
3829+
ASSERT_EQ(tiered_index->indexMetaDataCapacity(),
3830+
hnsw_index->indexMetaDataCapacity() +
3831+
tiered_index->frontendIndex->indexMetaDataCapacity());
38173832

38183833
// delete a vector to shrink data blocks
38193834
ASSERT_EQ(VecSimIndex_DeleteVector(tiered_index, 0), 1) << "Failed to delete vector 0";
@@ -3825,6 +3840,8 @@ TYPED_TEST(HNSWTieredIndexTestBasic, HNSWResize) {
38253840
ASSERT_EQ(tiered_index->getMainIndexGuardWriteLockCount(), resize_operations);
38263841
ASSERT_EQ(hnsw_index->indexSize(), blockSize);
38273842
ASSERT_EQ(hnsw_index->indexCapacity(), blockSize);
3843+
// meta data capacity should not shrink
3844+
ASSERT_EQ(hnsw_index->indexMetaDataCapacity(), 2 * blockSize);
38283845

38293846
// add this vector again and verify lock was acquired to resize
38303847
GenerateAndAddVector<TEST_DATA_T>(tiered_index, dim, 0);
@@ -3833,6 +3850,11 @@ TYPED_TEST(HNSWTieredIndexTestBasic, HNSWResize) {
38333850
ASSERT_EQ(tiered_index->getMainIndexGuardWriteLockCount(), resize_operations);
38343851
ASSERT_EQ(hnsw_index->indexSize(), blockSize + 1);
38353852
ASSERT_EQ(hnsw_index->indexCapacity(), 2 * blockSize);
3853+
ASSERT_EQ(hnsw_index->indexMetaDataCapacity(), 2 * blockSize);
3854+
ASSERT_EQ(tiered_index->frontendIndex->indexMetaDataCapacity(), 0);
3855+
ASSERT_EQ(tiered_index->indexMetaDataCapacity(),
3856+
hnsw_index->indexMetaDataCapacity() +
3857+
tiered_index->frontendIndex->indexMetaDataCapacity());
38363858

38373859
// add up to block size (count = 2 blockSize), the lock shouldn't be acquired because no resize
38383860
// is required
@@ -3843,4 +3865,9 @@ TYPED_TEST(HNSWTieredIndexTestBasic, HNSWResize) {
38433865
ASSERT_EQ(tiered_index->getMainIndexGuardWriteLockCount(), resize_operations);
38443866
ASSERT_EQ(hnsw_index->indexSize(), 2 * blockSize);
38453867
ASSERT_EQ(hnsw_index->indexCapacity(), 2 * blockSize);
3868+
ASSERT_EQ(hnsw_index->indexMetaDataCapacity(), 2 * blockSize);
3869+
ASSERT_EQ(tiered_index->frontendIndex->indexMetaDataCapacity(), 0);
3870+
ASSERT_EQ(tiered_index->indexMetaDataCapacity(),
3871+
hnsw_index->indexMetaDataCapacity() +
3872+
tiered_index->frontendIndex->indexMetaDataCapacity());
38463873
}

0 commit comments

Comments
 (0)