Skip to content

Seismic search optimizations #20

Open
chishui wants to merge 3 commits into
opensearch-project:mainfrom
chishui:seismic-search-optimizations-and-mmap
Open

Seismic search optimizations #20
chishui wants to merge 3 commits into
opensearch-project:mainfrom
chishui:seismic-search-optimizations-and-mmap

Conversation

@chishui

@chishui chishui commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Description

Done some optimization which could bring overall query performance by >30%

k = 10

  ┌──────────────────────┬─────────────────────┬─────────┐
  │        Config        │ CPU ms (mean ± std) │ Speedup │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ baseline             │ 127.7 ± 12.4        │ 1.00×   │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ + VisitedSet only    │ 116.6 ± 1.5         │ 1.10×   │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ + head-prefetch only │ 107.6 ± 8.9         │ 1.19×   │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ + hugepage only      │ 124.8 ± 6.0         │ 1.02×   │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ all three together   │ 91.9 ± 0.3          │ 1.39×   │
  └──────────────────────┴─────────────────────┴─────────┘

k = 100

  ┌──────────────────────┬─────────────────────┬─────────┐
  │        Config        │ CPU ms (mean ± std) │ Speedup │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ baseline             │ 353.6 ± 8.8         │ 1.00×   │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ + VisitedSet only    │ 347.4 ± 6.1         │ 1.02×   │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ + head-prefetch only │ 291.2 ± 5.6         │ 1.21×   │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ + hugepage only      │ 336.9 ± 3.9         │ 1.05×   │
  ├──────────────────────┼─────────────────────┼─────────┤
  │ all three together   │ 257.8 ± 1.7         │ 1.37×   │
  └──────────────────────┴─────────────────────┴─────────┘

Notes

  • head-prefetch is the dominant lever at both k (1.19× / 1.21×).
  • VisitedSet helps meaningfully at k=10 (1.10×) but washes out at k=100 (1.02×, within noise).
  • hugepage alone is small (1.02× / 1.05×) — borderline noise.
  • All three together (1.39× / 1.37×) exceed the levers stacked individually — they're synergistic: once the doc-scan is prefetch-bound, the dedup and page-walk wins compound rather than add.
  • "All three" is the exact config now committed (d704f61); the two dropped levers (doc-offset precompute, two-lane AVX512) were within noise and removing them cost only ~1%.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

chishui added 2 commits July 23, 2026 06:59
Apply search hot-path optimizations to SeismicIndex and
SeismicScalarQuantizedIndex, validated by an inter-process ablation on the
8.8M-doc corpus (~1.4x for float Seismic, ~1.06x for the 8-bit SQ variant):

- VisitedSet: bitset dedup with sparse per-query reset, replacing
  absl::flat_hash_set on the doc-scan critical path.
- head-prefetch (prefetch_vector_head): prefetch only the leading cache
  lines of the next doc row, bounding outstanding software prefetches.
- hugepage advise: MADV_HUGEPAGE/COLLAPSE the corpus arrays to cut
  TLB/page-walk cost on the random per-doc gather.
- sort_cluster_docs: sort doc ids ascending within each cluster so the
  per-doc gather is monotonic rather than random.

Ablation showed doc-offset precompute and two-lane AVX512 were within noise
individually, so they are not included.

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
The search hot path indexes VisitedSet by doc id with no per-candidate
bounds check (a branch there would erode the dedup optimization). That
makes "every doc id < num_vectors" a load-bearing invariant: unlike the
former hash set, an out-of-range id is an out-of-bounds write rather than
a safe insert.

Add InvertedListClusters::validate_doc_ids(num_docs), a single O(nnz) scan
that throws std::out_of_range on a stray id, and call it from both
SeismicIndex and SeismicScalarQuantizedIndex read_index() once clusters
and vectors are loaded. This fails loudly on a corrupt/mismatched index in
every build, off the hot path. A debug assert in VisitedSet::insert
documents the invariant at the point of use (no-op in release).

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
@chishui
chishui force-pushed the seismic-search-optimizations-and-mmap branch from 17417f6 to ce6a599 Compare July 23, 2026 07:00
Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
// smaller (1 bit/doc, ~1.1 MB at 8.8M docs) so far fewer distinct cache lines
// are touched, while the touched-word list lets a new query clear only the
// words it actually dirtied (sparse O(visited) reset, not O(n)).
class VisitedSet {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the detailed estimation on the 8.8M dataset. Do you think the current visited set can apply to all dataset scales? I'm worrying the bit set size from the super large dataset

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It scales with data size, so 11 MB for 88M, and we've benchmark at most 45M docs for a single host, seems OK to me, but I'll benchmark on 45M later.

@zirui-song-18 zirui-song-18 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In addition, I'm worried that the huge pages might introduce P99 spikes via NUMA auto-migration. Would you mind testing the P50 P90 P99 data before & after the optimization?

Comment on lines +40 to +45
void new_query() {
for (const size_t w : touched_) {
bits_[w] = 0;
}
touched_.clear();
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The new_query() sparse-reset is the most fragile point in this PR — it clears only touched_ words, relying on "a word is in touched_ iff it has a set bit" (maintained solely by if (prev==0) touched_.push_back(w)). I don't see a VisitedSet test (the only test change is transposed_scoring_matches_reference). Please add: (a) dedup within a query; (b) full reset across new_query; (c) two ids in the same 64-bit word push touched_ once and both clear; (d) resize() clears touched_; (e) boundaries 63/64/n-1. A regression here silently corrupts recall and is invisible to perf benchmarks.

Comment on lines +40 to +45
void new_query() {
for (const size_t w : touched_) {
bits_[w] = 0;
}
touched_.clear();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When k is larger than 100, new_query will take longer time clearing the touched. Can we have different strategy for the set? For example, when k <= 100, use the current one. If k > 1000, use the hash set?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's actually related to the doc count we visited, not the value of k, right?

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.

3 participants