michaeljmarshall commented on PR #16145: URL: https://github.com/apache/lucene/pull/16145#issuecomment-5818795844
I started thinking about this more last week. I am hopeful to get the design finalized and merged soon. It'll probably diverge from my initial proposal. Thanks for your comments and experiments @neoremind, they have been really helpful in clarifying my understanding of the problem and how we can solve it. First, I am not satisfied with the fact that `isRandom` is final and can therefore get out of sync with the underlying VMA's advice if a clone changes it, and I also don't like that this only benefited random advice but not the other types. At the most basic level, we call `prefetch` to minimize the likelihood of a subsequent major page fault on a read to the prefetched region. The goal of this PR is to improve the efficacy of the `prefetch` method. The naive (and original) implementation calls `MADV_WILLNEED` for all prefetch calls, but as https://github.com/apache/lucene/pull/13381 demonstrated, the 1-2us overhead is significant for the common case where the data is fully cached in RAM. #13381 added a power of 2 back off, however, as I argue below, the back off climbs too aggressively when we consider the cost of a probe vs the cost of a major page fault (discussed in this comment https://github.com/apache/lucene/pull/13381/changes#r1606102693). Let's cover the available levers: | Action | Cost if resident | Cost if not resident | Feedback on page cache state | |---|---|---|---| | `isLoaded()` (`mincore`) | ~1 µs | ~1 µs | yes, indicates whether every page in the region is warm | | `madvise(MADV_WILLNEED)` | 0, if isLoaded() called first, or ~1 µs | ~1 µs, and kicks off async readahead | No | | Skip both | ~0 | ~100 µs stall on the following read (NVMe) | No | The main challenge for us is limited feedback loops. The only tool available to get insight into the page cache is the `isLoaded()` call. Next, the current power of two back off skips prefetch calls number 3, 5–7, 9–15, 17–31, 33–63, and so on with an ever increasing gap. Given that the cost of a major page fault is ~100x of the call to `isLoaded()`, it seems we're too eager to skip the `isLoaded` call based on the relative cost of the probe vs a major page fault. We should only skip the `isLoaded` call when we are very confident the data is not in RAM. It also seems we're assuming that the memory pressure is essentially unchanging because we probe less frequently as long as the next power of 2 is a page cache hit. That doesn't seem like a safe assumption. Given the asymmetry of costs, I will investigate making the slow start even slower and adding an upper bound to the gap between the `isLoaded` calls. I intend to make the solution advice agnostic. Additional thoughts: * If we had another feedback loop, we might take a different path. For example, if at the start of a query we could determine that all of the data is RAM resident via a single isLoaded() call, perhaps we can skip all of the prefetch logic? * If we are running on a spinning disk, the cost of a major page fault is significantly higher and we might accept the overhead of more `isLoaded()` calls to avoid a major page fault. -- 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]
