From f0fd366003d0ebf065a33db06fd85ccd2c05ef65 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 22 Mar 2024 11:00:33 +0100 Subject: [PATCH 1/3] Set randomAccess=true on LOAD. --- lucene/core/src/java/org/apache/lucene/store/IOContext.java | 5 ++++- .../apache/lucene/store/MemorySegmentIndexInputProvider.java | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/store/IOContext.java b/lucene/core/src/java/org/apache/lucene/store/IOContext.java index 3bb37ae1d191..b387dc798297 100644 --- a/lucene/core/src/java/org/apache/lucene/store/IOContext.java +++ b/lucene/core/src/java/org/apache/lucene/store/IOContext.java @@ -64,7 +64,7 @@ public enum Context { public static final IOContext READ = new IOContext(false, false, false); - public static final IOContext LOAD = new IOContext(false, true, false); + public static final IOContext LOAD = new IOContext(false, true, true); public static final IOContext RANDOM = new IOContext(false, false, true); @@ -90,6 +90,9 @@ private IOContext(boolean readOnce, boolean load, boolean randomAccess) { if (readOnce && randomAccess) { throw new IllegalArgumentException("cannot be both readOnce and randomAccess"); } + if (load && randomAccess == false) { + throw new IllegalArgumentException("cannot be load but not randomAccess"); + } this.context = Context.READ; this.mergeInfo = null; this.readOnce = readOnce; diff --git a/lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java b/lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java index bcf58d95d015..e1c2940fdc1d 100644 --- a/lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java +++ b/lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java @@ -84,7 +84,8 @@ private final MemorySegment[] map( } final OptionalInt advice; - if (chunkSizePower < 21) { + if (preload || chunkSizePower < 21) { + // if preload is true, the advice is ignored // if chunk size is too small (2 MiB), disable madvise support (incorrect alignment): advice = OptionalInt.empty(); } else { From 6b09561a7367b9fbfadd1d7be271b8134485fbc8 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 22 Mar 2024 11:15:58 +0100 Subject: [PATCH 2/3] Javadocs --- lucene/core/src/java/org/apache/lucene/store/IOContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucene/core/src/java/org/apache/lucene/store/IOContext.java b/lucene/core/src/java/org/apache/lucene/store/IOContext.java index b387dc798297..f2c038d1bfde 100644 --- a/lucene/core/src/java/org/apache/lucene/store/IOContext.java +++ b/lucene/core/src/java/org/apache/lucene/store/IOContext.java @@ -54,7 +54,7 @@ public enum Context { * This flag is used for files that are a small fraction of the total index size and are expected * to be heavily accessed in random-access fashion. Some {@link Directory} implementations may * choose to load such files into physical memory (e.g. Java heap) as a way to provide stronger - * guarantees on query latency. + * guarantees on query latency. If this flag is set, then {@link #randomAccess} will be true. */ public final boolean load; From 62c708ef57138562d5a0595e3a1d34d57bcf8251 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Fri, 22 Mar 2024 11:24:35 +0100 Subject: [PATCH 3/3] Cleanup code and move IOContext mapping to Posix, as the constants we use are Poxix only. On Windows we can have a totally different way to madvise the kernel --- .../MemorySegmentIndexInputProvider.java | 31 ++----------- .../org/apache/lucene/store/NativeAccess.java | 19 +------- .../apache/lucene/store/NoopNativeAccess.java | 2 +- .../lucene/store/PosixNativeAccess.java | 44 ++++++++++++++++++- 4 files changed, 48 insertions(+), 48 deletions(-) diff --git a/lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java b/lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java index bcf58d95d015..c0ea1a11f5f0 100644 --- a/lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java +++ b/lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java @@ -23,8 +23,6 @@ import java.nio.channels.FileChannel.MapMode; import java.nio.file.Path; import java.nio.file.StandardOpenOption; -import java.util.OptionalInt; -import org.apache.lucene.store.IOContext.Context; import org.apache.lucene.util.Constants; import org.apache.lucene.util.Unwrappable; @@ -83,14 +81,6 @@ private final MemorySegment[] map( throw new IllegalArgumentException("File too big for chunk size: " + resourceDescription); } - final OptionalInt advice; - if (chunkSizePower < 21) { - // if chunk size is too small (2 MiB), disable madvise support (incorrect alignment): - advice = OptionalInt.empty(); - } else { - advice = mapContextToMadvise(context); - } - final long chunkSize = 1L << chunkSizePower; // we always allocate one more segments, the last one may be a 0 byte one @@ -108,29 +98,16 @@ private final MemorySegment[] map( } catch (IOException ioe) { throw convertMapFailedIOException(ioe, resourceDescription, segSize); } + // if preload apply it without madvise. + // if chunk size is too small (2 MiB), disable madvise support (incorrect alignment) if (preload) { segment.load(); - } else if (segSize > 0L && advice.isPresent()) { // not when preloading! - nativeAccess.madvise(segment, advice.getAsInt()); + } else if (chunkSizePower < 21) { + nativeAccess.madvise(segment, context); } segments[segNr] = segment; startOffset += segSize; } return segments; } - - private OptionalInt mapContextToMadvise(IOContext context) { - // Merging always wins and implies sequential access, because kernel is advised to free pages - // after use: - if (context.context == Context.MERGE) { - return OptionalInt.of(NativeAccess.POSIX_MADV_SEQUENTIAL); - } - if (context.randomAccess) { - return OptionalInt.of(NativeAccess.POSIX_MADV_RANDOM); - } - if (context.readOnce) { - return OptionalInt.of(NativeAccess.POSIX_MADV_SEQUENTIAL); - } - return OptionalInt.empty(); - } } diff --git a/lucene/core/src/java21/org/apache/lucene/store/NativeAccess.java b/lucene/core/src/java21/org/apache/lucene/store/NativeAccess.java index 41ed1f7b8a37..6ce1f73f2567 100644 --- a/lucene/core/src/java21/org/apache/lucene/store/NativeAccess.java +++ b/lucene/core/src/java21/org/apache/lucene/store/NativeAccess.java @@ -26,25 +26,8 @@ abstract class NativeAccess { private static final Logger LOG = Logger.getLogger(NativeAccess.class.getName()); - // these constants were extracted from glibc and macos header files - luckily they are the same: - - /** No further special treatment. */ - public static final int POSIX_MADV_NORMAL = 0; - - /** Expect random page references. */ - public static final int POSIX_MADV_RANDOM = 1; - - /** Expect sequential page references. */ - public static final int POSIX_MADV_SEQUENTIAL = 2; - - /** Will need these pages. */ - public static final int POSIX_MADV_WILLNEED = 3; - - /** Don't need these pages. */ - public static final int POSIX_MADV_DONTNEED = 4; - /** Invoke the {@code madvise} call for the given {@link MemorySegment}. */ - public abstract void madvise(MemorySegment segment, int advice) throws IOException; + public abstract void madvise(MemorySegment segment, IOContext context) throws IOException; /** * Return the NativeAccess instance for this platform. At moment we only support Linux and MacOS diff --git a/lucene/core/src/java21/org/apache/lucene/store/NoopNativeAccess.java b/lucene/core/src/java21/org/apache/lucene/store/NoopNativeAccess.java index a72c8eb0c0ca..2b6aea6f8236 100644 --- a/lucene/core/src/java21/org/apache/lucene/store/NoopNativeAccess.java +++ b/lucene/core/src/java21/org/apache/lucene/store/NoopNativeAccess.java @@ -22,5 +22,5 @@ final class NoopNativeAccess extends NativeAccess { @Override - public void madvise(MemorySegment segment, int advice) {} + public void madvise(MemorySegment segment, IOContext context) {} } diff --git a/lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java b/lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java index 7a1f601050f3..39c0e4129b8c 100644 --- a/lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java +++ b/lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java @@ -25,12 +25,30 @@ import java.lang.invoke.MethodHandle; import java.util.Locale; import java.util.logging.Logger; +import org.apache.lucene.store.IOContext.Context; @SuppressWarnings("preview") final class PosixNativeAccess extends NativeAccess { private static final Logger LOG = Logger.getLogger(PosixNativeAccess.class.getName()); + // these constants were extracted from glibc and macos header files - luckily they are the same: + + /** No further special treatment. */ + public static final int POSIX_MADV_NORMAL = 0; + + /** Expect random page references. */ + public static final int POSIX_MADV_RANDOM = 1; + + /** Expect sequential page references. */ + public static final int POSIX_MADV_SEQUENTIAL = 2; + + /** Will need these pages. */ + public static final int POSIX_MADV_WILLNEED = 3; + + /** Don't need these pages. */ + public static final int POSIX_MADV_DONTNEED = 4; + private final MethodHandle mh$posix_madvise; public PosixNativeAccess() { @@ -62,10 +80,17 @@ private static MethodHandle findFunction( } @Override - public void madvise(MemorySegment segment, int advice) throws IOException { + public void madvise(MemorySegment segment, IOContext context) throws IOException { + if (segment.byteSize() == 0L) { + return; // empty segments should be excluded, because they may have no address at all + } + final Integer advice = mapIOContext(context); + if (advice == null) { + return; // do nothing + } final int ret; try { - ret = (int) mh$posix_madvise.invokeExact(segment, segment.byteSize(), advice); + ret = (int) mh$posix_madvise.invokeExact(segment, segment.byteSize(), advice.intValue()); } catch (Throwable th) { throw new AssertionError(th); } @@ -79,4 +104,19 @@ public void madvise(MemorySegment segment, int advice) throws IOException { ret)); } } + + private Integer mapIOContext(IOContext ctx) { + // Merging always wins and implies sequential access, because kernel is advised to free pages + // after use: + if (ctx.context == Context.MERGE) { + return POSIX_MADV_SEQUENTIAL; + } + if (ctx.randomAccess) { + return POSIX_MADV_RANDOM; + } + if (ctx.readOnce) { + return POSIX_MADV_SEQUENTIAL; + } + return null; + } }