jimczi commented on issue #16044:
URL: https://github.com/apache/lucene/issues/16044#issuecomment-4722489013

   Thanks for the detailed benchmark — the `NativeThreadSet` analysis is 
genuinely useful. But I think motivation #1 (the mmap "page-fault storm") 
conflates three separate things, and the benchmark's mmap arm is configured to 
produce the very effect it attributes to mmap.
   
   **The mmap arm sets no read advice.** It maps and reads via 
`MemorySegment.copy` with no `posix_madvise`, which reproduces 
`MMapDirectory`'s *current default* `ReadAdvice.NORMAL` — deliberately no 
`madvise`, so kernel readahead stays on (`MMapDirectory.java:385-388`). Under a 
working set > RAM, that readahead is what inflates `pgmajfault`: each random 
access drags in a speculative window that wastes IOPS and evicts hot pages. 
`pread`/`FileChannel` don't, because they read exactly the requested bytes. So 
this is *mmap-with-readahead* vs *pread-without*, not mmap vs pread. (It's also 
worth noting `pread` cache-misses do the same disk reads but are accounted 
under block-I/O stats, not `pgmajfault` — so a `pgmajfault` drop after 
switching to NIOFS is partly just relabeling, not less I/O.)
   
   **`MADV_RANDOM` removes the over-read.** `ReadAdvice.RANDOM` issues 
`POSIX_MADV_RANDOM` (`VM_RAND_READ`), disabling fault-path readahead so a 
mapping reads the *same pages* as an equivalent `pread` — no eviction storm. 
Available today without changing Directory type: 
`-Dorg.apache.lucene.store.defaultReadAdvice=RANDOM`, or 
`dir.setReadAdvice(MMapDirectory.ADVISE_BY_CONTEXT)`. This was added (#13196) 
specifically "to improve paging logic … under memory pressure," and was the 
default until 10.3 (#15040).
   
   **The one real edge — and its fix.** For *large cold* reads, `pread` carries 
the length and submits one batched I/O, whereas `MADV_RANDOM` mmap faults 
page-by-page (serialized single-page I/Os) — more round-trips, scaling with 
read size and device latency. The right fix on the mmap side is **prefetch**, 
not a new Directory: `IndexInput.prefetch()` → `madvise(WILLNEED)` 
(`MemorySegmentIndexInput.java:354`) hands the kernel the range up front for 
one batched async read, recovering `pread`-equivalent batching while keeping 
random access readahead-free. Lucene already does this on the hot paths.
   
   **Could you re-run the mmap arm with `MADV_RANDOM` (and `MADV_WILLNEED` 
prefetch for the multi-page reads)?** I'd expect the under-pressure collapse to 
disappear and the batching gap to close — isolating how much of #1 is config 
vs. inherent.
   
   To be clear, none of this touches motivation #2: the `NativeThreadSet` 
monitor is a real `NIOFSDirectory` ceiling, and *that* is the legitimate basis 
for an FFI `pread` path.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to