From 37a8287a0b1c5482686bf9e3215cdf03ab07c921 Mon Sep 17 00:00:00 2001 From: Liyun Xiu Date: Fri, 4 Sep 2026 01:24:46 +0000 Subject: [PATCH] Keep the mapped CSR and the mapped index in separate members MmapIndex held one MmapFile for both mappings a borrowed buffer can point into, on the grounds that a native CSR corpus and a serialized index file are mutually exclusive sources. They are, but they are populated by unrelated paths and released at different times, and the single member made that hard to see: InvertedIndex::build() closes the mapping once its posting lists own their entries, which is only ever correct for the CSR side, while every mmap_index() assigns the index side. Split it into csr_mapping_ and index_mapping_. Both stay declared ahead of vectors_ and batch_spill_, so reverse-declaration-order destruction still outlives every borrower. No behaviour change. Signed-off-by: Liyun Xiu --- nsparse/disk_seismic_index_base.cpp | 4 ++-- nsparse/inverted_index.cpp | 8 ++++---- nsparse/mmap_index.h | 23 ++++++++++++---------- nsparse/seismic_index.cpp | 6 +++--- nsparse/seismic_scalar_quantized_index.cpp | 6 +++--- 5 files changed, 25 insertions(+), 22 deletions(-) diff --git a/nsparse/disk_seismic_index_base.cpp b/nsparse/disk_seismic_index_base.cpp index 3b4cc7b..d01e24f 100644 --- a/nsparse/disk_seismic_index_base.cpp +++ b/nsparse/disk_seismic_index_base.cpp @@ -194,9 +194,9 @@ void DiskSeismicIndexBase::load_mapped_payload(MmapCursor* cursor, // width mismatch before we commit. validate_mapped_payload(); - // mapped_file_ last: the summaries and the forward index borrow from it, + // index_mapping_ last: the summaries and the forward index borrow from it, // and moving it does not move the mapping. - mapped_file_ = std::move(mapped); + index_mapping_ = std::move(mapped); } } // namespace nsparse diff --git a/nsparse/inverted_index.cpp b/nsparse/inverted_index.cpp index d24abbf..214b27f 100644 --- a/nsparse/inverted_index.cpp +++ b/nsparse/inverted_index.cpp @@ -278,7 +278,7 @@ void InvertedIndex::build() { vectors_.reset(); // The posting lists own their entries, so nothing borrows from a mapped CSR // once the vectors are gone. - mapped_file_.close(); + csr_mapping_.close(); // Posting lists are already sorted by doc_id because // build_inverted_lists iterates documents in ascending order. @@ -471,14 +471,14 @@ InvertedIndex* InvertedIndex::mmap_index(const IndexHeader& header, throw_if_scores_are_incomplete(*lists, scores_size); // Committed only once everything parsed, so a corrupt file cannot leave a - // half-mapped index behind. mapped_file_ last: the arrays above borrow from - // it, and moving it does not move the mapping itself. + // half-mapped index behind. index_mapping_ last: the arrays above borrow + // from it, and moving it does not move the mapping itself. index->num_vectors_ = num_vectors; index->max_term_scores_ = std::move(scores); if (lists->size() > 0) { index->inverted_lists_ = std::move(lists); } - index->mapped_file_ = std::move(mmap_file); + index->index_mapping_ = std::move(mmap_file); return index.release(); } diff --git a/nsparse/mmap_index.h b/nsparse/mmap_index.h index 0e9173b..42cb109 100644 --- a/nsparse/mmap_index.h +++ b/nsparse/mmap_index.h @@ -51,21 +51,24 @@ class MmapIndex : public Index { } protected: - // The mapping borrowed buffers point into: a native CSR file via read_csr, - // or a serialized index file. Those sources are mutually exclusive, so one - // member serves both. + // The two mappings borrowed buffers can point into, kept apart because they + // are populated by unrelated paths and released independently: a native CSR + // corpus opened by read_csr's mapped residency, and a serialized index file + // opened by a derived mmap_index. // - // Borrowers do not reference it, so it must outlive them. Members are - // destroyed in reverse declaration order, hence this one first: do not - // reorder it past anything that borrows from it. - MmapFile mapped_file_; + // Borrowers do not reference either one, so both must outlive them. Members + // are destroyed in reverse declaration order, hence these first: do not + // reorder them past anything that borrows from them. + MmapFile csr_mapping_; + MmapFile index_mapping_; // Either residency: buffers owned when built or deserialized, borrowed from - // mapped_file_ when mapped. get_vectors() cannot tell the two apart. + // csr_mapping_ or index_mapping_ when mapped. get_vectors() cannot tell the + // cases apart. std::unique_ptr vectors_; // A batched build's spill, which its posting lists borrow from. Separate - // from mapped_file_ because the two coexist: the corpus may itself be a + // from csr_mapping_ because the two coexist: the corpus may itself be a // mapping vectors_ is still borrowing from. // // Its borrowers live in the derived class, destroyed before base members, @@ -155,7 +158,7 @@ class MmapIndex : public Index { nnz_size * element_size)); // Committed last, so a rejected file leaves the index untouched. - mapped_file_ = std::move(file); + csr_mapping_ = std::move(file); vectors_ = std::move(vectors); } }; diff --git a/nsparse/seismic_index.cpp b/nsparse/seismic_index.cpp index d0fd025..df3fe40 100644 --- a/nsparse/seismic_index.cpp +++ b/nsparse/seismic_index.cpp @@ -317,13 +317,13 @@ SeismicIndex* SeismicIndex::mmap_index(const IndexHeader& header, inv_list_writer.mmap_deserialize(&cursor); // Committed only once everything parsed, so a corrupt file cannot leave a - // half-mapped index behind. mapped_file_ last: the arrays above borrow from - // it, and moving it does not move the mapping itself. + // half-mapped index behind. index_mapping_ last: the arrays above borrow + // from it, and moving it does not move the mapping itself. index->clustered_inverted_lists = std::move(inv_list_writer.release()); if (vectors->num_vectors() > 0) { index->vectors_ = std::move(vectors); } - index->mapped_file_ = std::move(mmap_file); + index->index_mapping_ = std::move(mmap_file); return index.release(); } } // namespace nsparse \ No newline at end of file diff --git a/nsparse/seismic_scalar_quantized_index.cpp b/nsparse/seismic_scalar_quantized_index.cpp index 01f4b73..e46d737 100644 --- a/nsparse/seismic_scalar_quantized_index.cpp +++ b/nsparse/seismic_scalar_quantized_index.cpp @@ -400,14 +400,14 @@ SeismicScalarQuantizedIndex* SeismicScalarQuantizedIndex::mmap_index( inv_list_writer.mmap_deserialize(&cursor); // Committed only once everything parsed, so a corrupt file cannot leave a - // half-mapped index behind. mapped_file_ last: the arrays above borrow from - // it, and moving it does not move the mapping itself. + // half-mapped index behind. index_mapping_ last: the arrays above borrow + // from it, and moving it does not move the mapping itself. index->sq_ = sq; index->clustered_inverted_lists = std::move(inv_list_writer.release()); if (vectors->num_vectors() > 0) { index->vectors_ = std::move(vectors); } - index->mapped_file_ = std::move(mmap_file); + index->index_mapping_ = std::move(mmap_file); return index.release(); }