Skip to content

[Sparse ANN] Bump neural-sparse-cpp submodule for DiskSeismic madvise fix - #2005

Merged
chishui merged 1 commit into
opensearch-project:mainfrom
zirui-song-18:disk-seismic-mmap-madvise-fix
Sep 14, 2026
Merged

chishui merged 1 commit into
opensearch-project:mainfrom
zirui-song-18:disk-seismic-mmap-madvise-fix

Conversation

@zirui-song-18

Copy link
Copy Markdown
Contributor

Bump neural-sparse-cpp for the DiskSeismic mmap madvise fix (MADV_RANDOM for per_block), fixing a large memory-constrained latency regression. Details can be seen in: opensearch-project/neural-sparse-cpp#55

This makes the madvise hint a function of the index's access pattern instead of a single global default:

  • DiskSeismic (DiskSeismicIndex, DiskSeismicScalarQuantizedIndex) → MADV_RANDOM (point lookups; suppress readahead).
  • shared (SeismicIndex, SeismicScalarQuantizedIndex) → MADV_HUGEPAGE (broad scans; fewer TLB entries), unchanged.

NSPARSE_MMAP_ADVISE still overrides the choice (an empty value is treated as unset, so it can't silently reinstate the old default).

Measured impact

From the launch benchmark (i4i.16xlarge, MS MARCO V1, 12 GB page-cache cap, THP=[madvise], top_n=5, k′=20, recall ≈ 0.92), same index and queries:

arm advise p50 p99
disk (per_block) hugepage (old default) 6.529 ms 13.21 ms
disk (per_block) random (new default) 0.157 ms 0.296 ms
  • ~42× lower p50, ~45× lower p99. The old default was reading ~9,426 KiB per query for the ~53–1,055 KiB a query actually needs (~860× amplification over a 4 KiB page once RAID readahead compounds it). latency (0.157 ms) — the cap costs it nothing, using < 4.76 GB of page cache.
  • Resident memory for the disk arm drops from > 30.9 GB to 10.94 GB (−65%); the previously reported "per_block needs ~3× more RAM than shared" was largely a hugepage artifact (real ratio ~1.6×).

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

…p madvise fix (opensearch-project#55)

Signed-off-by: Zirui Song <zrsong@amazon.com>
@github-actions

Copy link
Copy Markdown

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 49c4275.

⛔ Hard block: Issues at High severity or above will block this PR from merging.

PathLineSeverityDescription
jni/external/neural-sparse-cpp1highGit submodule dependency updated to an unverified external commit (da0d43c -> 2da4627). This is a JNI native-code dependency that runs with full system privileges; the new commit hash must be verified against the upstream repository to confirm it matches the stated changelog purpose (mmap madvise fix) and contains no malicious native code.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 1 | Medium: 0 | Low: 0


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

@chishui chishui added the skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. label Sep 14, 2026
@codecov

codecov Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.21%. Comparing base (7a24f0a) to head (49c4275).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2005   +/-   ##
=========================================
  Coverage     84.20%   84.21%           
- Complexity     4239     4241    +2     
=========================================
  Files           318      318           
  Lines         14887    14887           
  Branches       2425     2425           
=========================================
+ Hits          12536    12537    +1     
  Misses         1485     1485           
+ Partials        866      865    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@chishui
chishui merged commit 88146e1 into opensearch-project:main Sep 14, 2026
119 of 120 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants