mirror of
https://github.com/facebookresearch/faiss.git
synced 2026-10-12 02:13:36 +00:00
fix: GPU CAGRA copyFrom host-memory lifetime dependency and add regression test (#4968)
Summary: This PR fixes https://github.com/facebookresearch/faiss/issues/4742, a crash where a `faiss::gpu::GpuIndexCagra` (and binary variant) created via `copyFrom` / `index_cpu_to_gpu` could retain shallow references to CPU-side dataset buffers. If the source CPU `IndexHNSWCagra` was destroyed, subsequent GPU searches could trigger CUDA illegal memory access / invalid device ordinal errors. The PR also fixes an out-of-bounds write in the dense kNN-graph construction in `GpuIndexCagra::copyFrom_ex` and adds a regression test that explicitly deletes the CPU source index before running GPU search. ## Changes - Ensure `GpuIndexCagra::copyFrom_ex` performs an owning copy of the training dataset into `GpuIndexCagra`-owned storage for: - Float32 - Float16 - Int8 (decoded) - Fix dense kNN-graph allocation in `GpuIndexCagra::copyFrom_ex` by using `resize()` (previously `reserve()` + indexed writes was UB). - Apply the same dataset-ownership fix to `GpuIndexBinaryCagra::copyFrom`. - Clear owned host storage on `reset()` for both float and binary CAGRA GPU indices. - Update CAGRA GPU unit tests to delete the source CPU index immediately after `copyFrom` and then run search, matching the reported repro scenario. ## Reproduction build GPU index → `copyTo` CPU → `copyFrom` back to GPU → destroy CPU index → search on GPU. ## Testing - Added/updated GPU regression tests in `TestGpuIndexCagra.cu` to cover “CPU source deleted after copyFrom”. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/4968 Reviewed By: alibeklfc Differential Revision: D97767287 Pulled By: mnorris11 fbshipit-source-id: 11f4892e4bb48cd4f99ba07779ed9b5d3860a596
This commit is contained in:
committed by
meta-codesync[bot]
parent
3030fe0635
commit
b923091c0d
@@ -28,6 +28,7 @@
|
||||
#include <faiss/gpu/GpuIndexCagra.h>
|
||||
#include <faiss/gpu/StandardGpuResources.h>
|
||||
#include <faiss/gpu/utils/StaticUtils.h>
|
||||
#include <cstdio>
|
||||
#include <faiss/gpu/impl/BinaryCuvsCagra.cuh>
|
||||
#include <faiss/gpu/utils/CopyUtils.cuh>
|
||||
|
||||
@@ -280,6 +281,10 @@ void GpuIndexBinaryCagra::copyFrom(const faiss::IndexBinaryHNSWCagra* index) {
|
||||
IndexBinaryFlat* flat_storage =
|
||||
dynamic_cast<IndexBinaryFlat*>(index->storage);
|
||||
FAISS_ASSERT(flat_storage);
|
||||
fprintf(stderr,
|
||||
"WARNING: GpuIndexBinaryCagra::copyFrom uses non-owning CPU "
|
||||
"storage. Keep the source IndexBinaryHNSWCagra alive for the "
|
||||
"lifetime of the GpuIndexBinaryCagra.\n");
|
||||
|
||||
auto hnsw = index->hnsw;
|
||||
// copy level 0 to a dense knn graph matrix
|
||||
|
||||
@@ -25,6 +25,7 @@
|
||||
#include <faiss/gpu/GpuIndexCagra.h>
|
||||
#include <faiss/gpu/StandardGpuResources.h>
|
||||
#include <cstddef>
|
||||
#include <cstdio>
|
||||
#include <faiss/gpu/impl/CuvsCagra.cuh>
|
||||
#include <optional>
|
||||
#include <type_traits>
|
||||
@@ -315,9 +316,10 @@ void GpuIndexCagra::copyFrom_ex(
|
||||
GpuIndex::copyFrom(index);
|
||||
|
||||
auto hnsw = index->hnsw;
|
||||
|
||||
// copy level 0 to a dense knn graph matrix
|
||||
std::vector<idx_t> knn_graph;
|
||||
knn_graph.reserve(index->ntotal * hnsw.nb_neighbors(0));
|
||||
knn_graph.resize(index->ntotal * hnsw.nb_neighbors(0));
|
||||
|
||||
#pragma omp parallel for
|
||||
for (size_t i = 0; i < index->ntotal; ++i) {
|
||||
@@ -334,6 +336,10 @@ void GpuIndexCagra::copyFrom_ex(
|
||||
auto base_index = dynamic_cast<IndexFlat*>(index->storage);
|
||||
FAISS_ASSERT(base_index);
|
||||
auto dataset = base_index->get_xb();
|
||||
fprintf(stderr,
|
||||
"WARNING: GpuIndexCagra::copyFrom uses non-owning CPU storage. "
|
||||
"Keep the source IndexHNSWCagra alive for the lifetime of the "
|
||||
"GpuIndexCagra.\n");
|
||||
|
||||
index_ = std::make_shared<CuvsCagra<float>>(
|
||||
this->resources_.get(),
|
||||
@@ -348,7 +354,11 @@ void GpuIndexCagra::copyFrom_ex(
|
||||
} else if (numeric_type == NumericType::Float16) {
|
||||
auto base_index = dynamic_cast<IndexScalarQuantizer*>(index->storage);
|
||||
FAISS_ASSERT(base_index);
|
||||
auto dataset = (half*)base_index->codes.data();
|
||||
auto dataset = reinterpret_cast<half*>(base_index->codes.data());
|
||||
fprintf(stderr,
|
||||
"WARNING: GpuIndexCagra::copyFrom uses non-owning CPU storage. "
|
||||
"Keep the source IndexHNSWCagra alive for the lifetime of the "
|
||||
"GpuIndexCagra.\n");
|
||||
|
||||
index_ = std::make_shared<CuvsCagra<half>>(
|
||||
this->resources_.get(),
|
||||
@@ -364,9 +374,11 @@ void GpuIndexCagra::copyFrom_ex(
|
||||
auto base_index = dynamic_cast<IndexScalarQuantizer*>(index->storage);
|
||||
FAISS_ASSERT(base_index);
|
||||
auto dataset = (uint8_t*)base_index->codes.data();
|
||||
fprintf(stderr,
|
||||
"WARNING: GpuIndexCagra::copyFrom uses non-owning CPU storage. "
|
||||
"Keep the source IndexHNSWCagra alive for the lifetime of the "
|
||||
"GpuIndexCagra.\n");
|
||||
|
||||
// decode what was encoded by Quantizer8bitDirectSigned in
|
||||
// ScalarQuantizer
|
||||
int8_t* decoded_train_dataset = new int8_t[index->ntotal * index->d];
|
||||
for (int i = 0; i < index->ntotal * this->d; i++) {
|
||||
decoded_train_dataset[i] = dataset[i] - 128;
|
||||
|
||||
@@ -28,6 +28,7 @@
|
||||
#include <faiss/gpu/GpuIndexIVFPQ.h>
|
||||
|
||||
#include <variant>
|
||||
#include <vector>
|
||||
#include "faiss/Index.h"
|
||||
|
||||
namespace faiss {
|
||||
|
||||
@@ -664,7 +664,7 @@ void copyFromTest(faiss::MetricType metric, double expected_recall) {
|
||||
res.noTempMemory();
|
||||
|
||||
// convert to gpu index
|
||||
faiss::gpu::GpuIndexCagra copiedGpuIndex(&res, cpuIndex.d, metric);
|
||||
faiss::gpu::GpuIndexCagra copiedGpuIndex(&res, opt.dim, metric);
|
||||
copiedGpuIndex.copyFrom(&cpuIndex);
|
||||
|
||||
// train gpu index
|
||||
@@ -782,7 +782,7 @@ void copyFromTestFP16(faiss::MetricType metric, double expected_recall) {
|
||||
res.noTempMemory();
|
||||
|
||||
// convert to gpu index
|
||||
faiss::gpu::GpuIndexCagra copiedGpuIndex(&res, cpuIndex.d, metric);
|
||||
faiss::gpu::GpuIndexCagra copiedGpuIndex(&res, opt.dim, metric);
|
||||
copiedGpuIndex.copyFrom_ex(&cpuIndex, faiss::NumericType::Float16);
|
||||
|
||||
// train gpu index
|
||||
@@ -795,7 +795,7 @@ void copyFromTestFP16(faiss::MetricType metric, double expected_recall) {
|
||||
// faiss::gpu::GpuIndexCagra gpuIndex(&res, opt.dim, metric, config);
|
||||
// gpuIndex.train(opt.numTrain, trainVecs.data());
|
||||
|
||||
faiss::gpu::GpuIndexCagra gpuIndex(&res, cpuIndex.d, metric, config);
|
||||
faiss::gpu::GpuIndexCagra gpuIndex(&res, opt.dim, metric, config);
|
||||
|
||||
// Create half vector
|
||||
std::vector<__half> trainVecs_half(trainVecs.size());
|
||||
|
||||
Reference in New Issue
Block a user