Skip to content

Fix/exec double count followup#6907

Open
brendanclement wants to merge 3 commits into
lance-format:mainfrom
brendanclement:fix/exec-double-count-followup
Open

Fix/exec double count followup#6907
brendanclement wants to merge 3 commits into
lance-format:mainfrom
brendanclement:fix/exec-double-count-followup

Conversation

@brendanclement
Copy link
Copy Markdown
Contributor

Addresses Weston's review comments on #6799

  • Revert: drop setup-time Instant::now() brackets that captured I/O time. I wrapped setup .awaits in MapIndexExec, FlatMatchFilterExec, and FlatMatchQueryExec with Instant::now() to add_duration(start.elapsed()) to attribute one-shot setup work to elapsed_compute. Weston pointed out those brackets capture I/O wait, not CPU.
  • Replace InstrumentedChildInputStream with inline RAII timer in .then() bodies, Deletes the InstrumentedChildInputStream helper struct.
  • Extend flat_bm25 scoring timer to pre- and post-await sync work In flat_bm25_search_stream_with_metrics the single scoring_start region only covered initialize_scorer + flat_bm25_score.

Testing

Benchmarked (1M-row synthetic dataset with BTREE + INVERTED + IVF_PQ indexes, identical analyze plan queries on each side). Comparing against the commit prior to this work

Scenario Node Before After Notes
KNN + BTREE scalar prefilter KNNVectorDistance (520 rows) 1.39 ms 112 µs ~12× smaller — the original double-count fix preserved
KNN + FTS post-filter FlatMatchFilter (0 rows) 10.95 ms 42 µs ~262× smaller
KNN + FTS prefilter FlatMatchQuery (2.52 K rows) 848 µs 79.08 ms ~93× larger; spawn_cpu tokenize CPU now correctly attributed
KNN + FTS prefilter (control) MatchQuery (untouched code) 3.80 ms 4.63 ms noise only

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions
Copy link
Copy Markdown
Contributor

ACTION NEEDED
Lance follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

For details on the error please inspect the "PR Title Check" action.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 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.

1 participant