Skip to content

MB-71397: Fix the output of Size API of VectorIndex#408

Open
CascadingRadium wants to merge 16 commits into
masterfrom
fixSize
Open

MB-71397: Fix the output of Size API of VectorIndex#408
CascadingRadium wants to merge 16 commits into
masterfrom
fixSize

Conversation

@CascadingRadium
Copy link
Copy Markdown
Member

@CascadingRadium CascadingRadium commented May 8, 2026

  • Implement a full recursive size calculation for the vector index, accounting for all owned sub-structures: the index params, the ID mapping, the exclude bitmap etc.
  • The underlying FAISS index memory is excluded from the calculation since it is memory-mapped and not RAM-resident.
  • Size reported in the VectorOptimize API of bleve :
    • Before:
      • 0.000152587890625 MB
    • After:
      • 171.66588592529297 MB
  • pprof output:
$ go tool pprof go_mem.pprof
File: bleve_query
Build ID: 8f0bd7de29dc423c4830da78a30cbbc3ac5025fb
Type: inuse_space
Time: 2026-05-11 17:13:34 IST
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 174.07MB, 99.14% of 175.57MB total
Dropped 22 nodes (cum <= 0.88MB)
Showing top 10 nodes out of 18

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts how vector-index memory usage is reported by VectorIndex.Size(), avoiding FAISS Size() calls that are misleading under mmap, and additionally accounting for wrapper-side structures like ID mappings and exclusion bitmaps.

Changes:

  • Reworked vectorIndexWrapper.Size() to sum sizes from the FAISS index wrapper, idMapping, and exclusion bitmap, removing the previously cached vecIndexSize.
  • Added approximate sizing helpers for idMapping and bitmap using reflect-based static sizing plus backing-slice contributions.
  • Updated FAISS index wrapper size() implementations (float32, GPU float32, binary IVF) to avoid calling FAISS Size() under mmap and instead return a reflect-based static size.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
faiss_vector_wrapper.go Adds reflect-based static sizing, introduces bitmap.size()/idMapping.size(), and updates VectorIndex.Size() to aggregate sizes.
faiss_vector_posting.go Removes initialization of the cached vecIndexSize in InterpretVectorIndex().
faiss_vector_index_float32.go Changes FAISS float32 index size() to return reflect-based static size instead of idx.Size().
faiss_vector_index_gpu_float32.go Changes GPU float32 index size() to return reflect-based static size instead of cpuIdx.Size().
faiss_vector_index_bivf.go Changes binary IVF index size() to return reflect-based static size instead of binary.Size()+backing.Size().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread faiss_vector_wrapper.go Outdated
Comment thread faiss_vector_wrapper.go
Comment thread faiss_vector_index_float32.go Outdated
Comment thread faiss_vector_index_gpu_float32.go
Comment thread faiss_vector_index_bivf.go
capemox
capemox previously approved these changes May 11, 2026
CascadingRadium and others added 4 commits May 11, 2026 14:46
Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comment thread faiss_vector_wrapper.go Outdated
Comment thread faiss_vector_wrapper.go
Comment thread faiss_vector_index.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@CascadingRadium CascadingRadium requested a review from Copilot May 11, 2026 11:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Comment thread faiss_vector_index.go Outdated
Comment thread faiss_vector_wrapper.go
Comment thread faiss_vector_wrapper.go Outdated
Comment thread faiss_vector_request_batcher.go
@Thejas-bhat
Copy link
Copy Markdown
Member

looks good, but can you attach the MB associated with this?

Base automatically changed from gpuIVFSq8 to master May 14, 2026 09:24
@CascadingRadium CascadingRadium dismissed capemox’s stale review May 14, 2026 09:24

The base branch was changed.

@CascadingRadium CascadingRadium changed the title Fix the output of Size API of VectorIndex MB-71397: Fix the output of Size API of VectorIndex May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants