Select mmap madvise by index type: MADV_RANDOM for DiskSeismic, not a global hugepage default

nsparse/disk_seismic_index.cpp

@@ -53,7 +53,7 @@ DiskSeismicIndex* DiskSeismicIndex::mmap_index(const IndexHeader& header,
     throw_if_null(index_file, "index_file must not be null");
     auto index = std::make_unique<DiskSeismicIndex>(header.dimension);
 
-    MmapFile mmap_file(std::string{index_file});
+    MmapFile mmap_file = map_index_file(index_file);
     MmapCursor cursor(mmap_file.data(), mmap_file.size());
     cursor.skip(pos);
 

nsparse/disk_seismic_index_base.h

@@ -12,6 +12,7 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <string>
 #include <vector>
 
 #include "nsparse/cluster/inverted_list_clusters.h"
@@ -117,6 +118,15 @@ class DiskSeismicIndexBase : public MmapIndex, public IndexIO {
     // reads its own extra header first, then calls this.
     void load_mapped_payload(MmapCursor* cursor, MmapFile&& mapped);
 
+    // Maps a disk index file for querying. Every disk index is point-accessed
+    // (~k' small inline-forward blocks per query), so the mapping must use
+    // MADV_RANDOM. Concrete mmap_index() factories map through here so a new
+    // disk type cannot silently fall back to the scan/hugepage default.
+    static MmapFile map_index_file(const char* index_file) {
+        return MmapFile(std::string{index_file},
+                        MmapFile::AccessPattern::kPointLookup);
+    }
+
     // Borrowed from by score_summaries_transposed / the inline forward index, so
     // the concrete validate_mapped_payload can inspect their widths.
     std::vector<InvertedListClusters> clustered_inverted_lists;

nsparse/disk_seismic_scalar_quantized_index.cpp

@@ -143,7 +143,7 @@ DiskSeismicScalarQuantizedIndex* DiskSeismicScalarQuantizedIndex::mmap_index(
     auto index =
         std::make_unique<DiskSeismicScalarQuantizedIndex>(header.dimension);
 
-    MmapFile mmap_file(std::string{index_file});
+    MmapFile mmap_file = map_index_file(index_file);
     MmapCursor cursor(mmap_file.data(), mmap_file.size());
     cursor.skip(pos);
 

nsparse/utils/mmap_file.h

@@ -49,8 +49,31 @@ namespace nsparse {
 // point their data structures directly into the returned bytes.
 class MmapFile {
 public:
+    // How the region is read, which picks the default madvise hint below.
+    // kScan expects broad reads over one forward index (MADV_HUGEPAGE, fewer
+    // TLB entries); kPointLookup expects scattered small reads (MADV_RANDOM, no
+    // readahead) -- what DiskSeismic's ~k' inline blocks per query need.
+    // NSPARSE_MMAP_ADVISE overrides this.
+    enum class AccessPattern { kScan, kPointLookup };
+
+    // The madvise mode for `access`, or `env` verbatim when it is non-null
+    // (NSPARSE_MMAP_ADVISE overrides the per-type default). Pure so the choice
+    // is unit-testable without a real mapping.
+    static std::string resolve_advise(AccessPattern access, const char* env) {
+        // An empty NSPARSE_MMAP_ADVISE (set but "") is treated as unset, so it
+        // does not silently override the per-type default with the "hugepage"
+        // fallback below.
+        if (env != nullptr && *env != '\0') {
+            return std::string(env);
+        }
+        return access == AccessPattern::kPointLookup ? "random" : "hugepage";
+    }
+
     MmapFile() = default;
-    explicit MmapFile(const std::string& path) { open(path); }
+    explicit MmapFile(const std::string& path,
+                      AccessPattern access = AccessPattern::kScan) {
+        open(path, access);
+    }
     ~MmapFile() { close(); }
 
     MmapFile(const MmapFile&) = delete;
@@ -67,7 +90,8 @@ class MmapFile {
     const uint8_t* data() const { return data_; }
     size_t size() const { return size_; }
 
-    void open(const std::string& path) {
+    void open(const std::string& path,
+              AccessPattern access = AccessPattern::kScan) {
         close();
 #if defined(_WIN32)
         file_ = CreateFileA(path.c_str(), GENERIC_READ, FILE_SHARE_READ, nullptr,
@@ -116,8 +140,11 @@ class MmapFile {
             return;  // empty file: leave data_ null
         }
 
-        const char* advise = std::getenv("NSPARSE_MMAP_ADVISE");
-        std::string mode = (advise != nullptr) ? advise : "hugepage";
+        // NSPARSE_MMAP_ADVISE overrides everything; otherwise the access
+        // pattern picks the default: point lookups (DiskSeismic) suppress
+        // readahead (MADV_RANDOM), scans collapse to huge pages.
+        const std::string mode =
+            resolve_advise(access, std::getenv("NSPARSE_MMAP_ADVISE"));
 
         // "hugetlb": copy the file into an anonymous MAP_HUGETLB region so the
         // index data is backed by real, pre-reserved 2 MiB pages (vm.nr_hugepages
@@ -172,9 +199,9 @@ class MmapFile {
         //   - MADV_HUGEPAGE lets the kernel back the region with 2 MiB pages,
         //     cutting TLB entries ~512x on the random-access dot-product path,
         //     at the cost of readahead the huge pages imply.
-        // Which wins is size-dependent, so the hint is selectable at runtime via
-        // NSPARSE_MMAP_ADVISE = "hugepage" | "random" | "normal" | "hugetlb"
-        // (default: hugepage, best for the large indexes this class targets).
+        // Which wins depends on the access pattern (see AccessPattern), so the
+        // default is per index type; NSPARSE_MMAP_ADVISE overrides it with
+        // "hugepage" | "random" | "normal" | "hugetlb".
         if (mode == "random") {
             ::madvise(addr, size_, MADV_RANDOM);
         } else if (mode == "normal") {

tests/CMakeLists.txt

@@ -24,6 +24,7 @@ set(NSPARSE_TEST_SRC
     inverted_lists_test.cpp
     kmeans_utils_test.cpp
     mmap_cursor_test.cpp
+    mmap_file_test.cpp
     mmap_index_test.cpp
     offset_width_test.cpp
     prefetch_test.cpp

tests/mmap_file_test.cpp

@@ -0,0 +1,82 @@
+/**
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+#include "nsparse/utils/mmap_file.h"
+
+#include <gtest/gtest.h>
+
+#include <cstdint>
+#include <cstring>
+#include <filesystem>
+#include <fstream>
+#include <string>
+#include <vector>
+
+namespace nsparse {
+namespace {
+
+using AccessPattern = MmapFile::AccessPattern;
+
+// The per-type default: DiskSeismic (point lookups of ~k' small inline blocks)
+// must suppress readahead with MADV_RANDOM, while the scanned shared forward
+// index collapses to huge pages. A regression here silently reinstates the
+// ~860x read amplification that makes per_block look slower than shared.
+TEST(MmapFileAdvise, DefaultsByAccessPattern) {
+    EXPECT_EQ(MmapFile::resolve_advise(AccessPattern::kPointLookup, nullptr),
+              "random");
+    EXPECT_EQ(MmapFile::resolve_advise(AccessPattern::kScan, nullptr),
+              "hugepage");
+}
+
+// NSPARSE_MMAP_ADVISE overrides the per-type default, for either pattern.
+TEST(MmapFileAdvise, EnvOverridesEitherPattern) {
+    for (const char* env : {"hugepage", "random", "normal", "hugetlb"}) {
+        EXPECT_EQ(MmapFile::resolve_advise(AccessPattern::kPointLookup, env),
+                  env);
+        EXPECT_EQ(MmapFile::resolve_advise(AccessPattern::kScan, env), env);
+    }
+}
+
+// An empty env var (set but "") is treated as unset, not as an override that
+// would silently force the "hugepage" fallback and undo DiskSeismic's default.
+TEST(MmapFileAdvise, EmptyEnvIsTreatedAsUnset) {
+    EXPECT_EQ(MmapFile::resolve_advise(AccessPattern::kPointLookup, ""),
+              "random");
+    EXPECT_EQ(MmapFile::resolve_advise(AccessPattern::kScan, ""), "hugepage");
+}
+
+// The access-pattern hint is advisory: the mapping returns the file's bytes
+// unchanged under either pattern.
+TEST(MmapFileAdvise, MapsSameBytesUnderEitherPattern) {
+    const std::filesystem::path path =
+        std::filesystem::temp_directory_path() / "nsparse_mmap_advise_test.bin";
+    std::vector<uint8_t> payload(8192);
+    for (size_t i = 0; i < payload.size(); ++i) {
+        payload[i] = static_cast<uint8_t>(i * 7 + 1);
+    }
+    {
+        std::ofstream out(path, std::ios::binary | std::ios::trunc);
+        out.write(reinterpret_cast<const char*>(payload.data()),
+                  static_cast<std::streamsize>(payload.size()));
+    }
+
+    for (const AccessPattern access :
+         {AccessPattern::kPointLookup, AccessPattern::kScan}) {
+        MmapFile file(path.string(), access);
+        ASSERT_EQ(file.size(), payload.size());
+        ASSERT_NE(file.data(), nullptr);
+        EXPECT_EQ(std::memcmp(file.data(), payload.data(), payload.size()), 0);
+    }
+
+    std::error_code ignored;
+    std::filesystem::remove(path, ignored);
+}
+
+}  // namespace
+}  // namespace nsparse