microsoft / microsoft/monodex

Every chunk read fetches the embedding vector and discards it

Open
#87 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
17
Forks
5
PR merge metrics
No merged PRs in 30d

Description

Every read through collect_rows in src/engine/storage/arrow.rs issues table.query().only_if(predicate).execute() with no projection. LanceDB defaults to Select::All, whose own doc comment says it is always slower than naming the columns you need, so each of these reads pulls all 22 columns off disk. That includes the 768-dimension embedding (about 3KB per row) and the full chunk text, and every caller then parses the batch with parse_chunk_row, which does not read the vector at all. The comment sitting at the top of that function says so outright: "vector is intentionally not read here; ChunkRow does not carry the embedding." The intent was already right; the query just never expressed it.

Lance is a columnar format, so this is close to pure waste. Six call sites go through the helper:

get_chunks_by_row_ids, get_chunks_by_file_id, get_chunks_by_file_ids, get_chunks_for_label, get_chunks_by_row_ids_for_label, and the label-metadata read in src/engine/storage/labels.rs.

The worst of them is get_chunks_for_label, which the label-reassignment cleanup calls with no ordinal filter at the end of every crawl at a new commit. On @davidh233's 186k-chunk repository that materializes every row carrying the label, roughly 1.6GB of vectors and text, to decide which few thousand rows are stale. Their measurements put label cleanup at 65 to 91s. Batching the writes (#88) does not touch this part, and I suspect the unprojected read is a large share of what remains.

Proposal

Give collect_rows a column list and pass one from every call site. The natural shape is for each parse function to declare the columns it reads, so the projection and the parser cannot drift apart. parse_chunk_row needs 21 of the 22 columns; the one it never touches is the expensive one.

A narrower version is to project only get_chunks_for_label, since that is the call site with a measurement attached. I would rather do all six: the change is mechanical, the same waste is present in the FTS hydration path where it costs search latency rather than crawl time, and leaving five of six unprojected preserves the pattern that produced this.

Two things worth saying about scope. This is not a substitute for scalar indexes (#89); projection reduces bytes read per scan, indexes reduce how much gets scanned, and the label-cleanup path pays both costs today. And it assumes no current caller wants the vector back from these helpers, which is true as the code stands, but if someone is planning work that hydrates vectors through this path, say so before the projection is hardcoded.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with collect_rows in src/engine/storage/arrow.rs and inspect parse_chunk_row to identify the columns it reads. Trace the six listed call sites, including the label-metadata read in src/engine/storage/labels.rs, and give each query an explicit projection. Done means these reads no longer fetch the unused embedding while preserving existing ChunkRow behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases, performance
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.