uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2016638471
I would still like to see some benchmark before merging this. Adding the
enum for the MADV constants can be a followup.
--
This is an automated message from the Apache Git Service.
rmuir commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2016627004
I agree, wasn't trying to hold up the change, was just thinking out loud!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub a
ChrisHegarty commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2016589340
> At a later stage we can ...
>
> I can do this later. OK?
This seems quite reasonable to me. Native access should be as close to a
wrapper around the native functions
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2016496447
Let's meet in the center:
- I will add a public enum `PosixMFAdvise` to store package with enum
constants named as the POSIX_MADV / POSIX_FADV constants (they are same, just
differ
rmuir commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2016483109
> We need an abstraction anyways, because not all platforms have same
constants.
Things can have the same names at least.
--
This is an automated message from the Apache Git Servi
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2016481753
> It's not the fault of the PR, but it's almost impossible for me to follow
what-happens-when thru the iocontext. Maybe we can overhaul it in trunk... I
don't think we should add our
rmuir commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2016455406
It's not the fault of the PR, but it's almost impossible for me to follow
what-happens-when thru the iocontext. Maybe we can overhaul it in trunk... I
don't think we should add our own lay
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1536612333
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -124,6 +142,7 @@ public int hashCode() {
result = prime * result + ((flushInfo == null) ?
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1536612274
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -124,6 +142,7 @@ public int hashCode() {
result = prime * result + ((flushInfo == null) ?
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1536595324
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -124,6 +142,7 @@ public int hashCode() {
result = prime * result + ((flushInfo == null) ?
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2015347052
Do we have any benchmarks by Mike? Some on memory restricted devices. An
idea would be a limited VM or by using the memory hog in Mike's benchmark.
--
This is an automated message f
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535773522
##
gradle/testing/defaults-tests.gradle:
##
@@ -132,6 +132,8 @@ allprojects {
if
(rootProject.vectorIncubatorJavaVersions.contains(rootProject.runtimeJavaVe
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2015341476
> > But when we backport this to 9.x, I tend to add another system property
to disable this feature.
>
> Why is this extra property needed? And not just enable madvise for the
ChrisHegarty commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2015293871
> But when we backport this to 9.x, I tend to add another system property to
disable this feature.
Why is this extra property needed? And not just enable madvise for the
Memo
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535725315
##
gradle/testing/defaults-tests.gradle:
##
@@ -132,6 +132,8 @@ allprojects {
if
(rootProject.vectorIncubatorJavaVersions.contains(rootProject.runtimeJava
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2015267061
I added more documentation and improved the following:
- the warning prints the correct command line based on how Lucene is used in
modularized or non-modularized code
- added th
ChrisHegarty commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2015261869
> I agree, here it triggers platform specific madvise(). We may add mocks
for testing, but I am not sure if it is worth. Maybe the easiest would be to
extend IOContext at a later ti
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2015178992
I added a basic test that verifies that we have madvise support enabled on
Linux and Macos. This also added
One question: In Lucene 9.x we had some system properties to disable
jpountz commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2015086424
I'm curious if you've already thought of how we could update the `IOContext`
on our merge instances. If I'm not mistaken, given how reader pooling works in
`IndexWriter`, if you use a `S
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2015060974
> > ... Additionally, I assume we can have unit tests for NativeAccess
directly. Lemme take a quick look at this.
>
> Dammit! I forgot about how our code is organised. Over in E
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535535951
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
rmuir commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535491072
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
rmuir commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535488782
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
rmuir commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535485315
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -124,6 +142,7 @@ public int hashCode() {
result = prime * result + ((flushInfo == null) ? 0 :
ChrisHegarty commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014959683
>... Additionally, I assume we can have unit tests for NativeAccess
directly. Lemme take a quick look at this.
Dammit! I forgot about how our code is organised. Over in Elasti
rmuir commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535484417
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -44,24 +44,32 @@ public enum Context {
/** This flag indicates that the file will be opened, the
ChrisHegarty commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014949144
> We can decide if we want to backport it (should not be too hard). If yes,
I will only backport to Java 21 (not 20 or 19).
++ Java 21 is fine.
> I will add some gette
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014920137
It looks like we are close for making this ready to be added to main branch.
We can decide if we want to backport it (should not be too hard). If yes, I
will only backport to Java 21
ChrisHegarty commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014875051
>..
> It is always a good idea to run one of those tests with
`-Ptests.verbose=true` to see if the INFO mesage about madvise is enabled is
printed on console:
>
> ```
>
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014868082
> > ./gradlew :lucene:core:test -Ptests.directory=MMapDirectory
>
> All tests pass on my Mac M2.
It is always a good idea to run one of those tests with
`-Ptests.verbose=
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014854516
> > > One thing: Should we remove the NoopNativeAccess class and just add a
null check?
> >
> >
> > Null is a little easier to reason about in the code at the use-site,
sin
ChrisHegarty commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014855899
> ./gradlew :lucene:core:test -Ptests.directory=MMapDirectory
All tests pass on my Mac M2.
Native constants are same as yours.
```
$ xcrun --show-sdk-path
/Libr
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014843058
> > One thing: Should we remove the NoopNativeAccess class and just add a
null check?
>
> Null is a little easier to reason about in the code at the use-site, since
the reader
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014839549
It would be good if you always run tests with `./gradlew :lucene:core:test
-Ptests.directory=MMapDirectory`
This will use MMapDir for all tests. I have no macos machine, if some
ChrisHegarty commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014837581
> One thing: Should we remove the NoopNativeAccess class and just add a null
check?
Null is a little easier to reason about in the code at the use-site, since
the reader is s
ChrisHegarty commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014833076
I took another run at static final ;-).
https://github.com/uschindler/lucene/pull/5 (if we still don't want it, then
I'll drop it)
--
This is an automated message from the Apache
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014829943
Thanks for the PR contributions. You may also commit directly to this
branch, unless the change is a complete rewrite :-)
--
This is an automated message from the Apache Git Service
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535372859
##
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##
@@ -92,10 +110,22 @@ private final MemorySegment[] map(
}
i
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2014791894
I refactored the code a bit:
- The POSIX constants are now part of PosixNativeAcess class
- The abstract method madvise() now only takes a `MemorySegment` and the
`IOContext`. So
jpountz commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535351343
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -54,14 +60,16 @@ public enum Context {
public static final IOContext DEFAULT = new IOContext
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535348590
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -54,14 +60,16 @@ public enum Context {
public static final IOContext DEFAULT = new IOCont
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535344579
##
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##
@@ -92,10 +110,22 @@ private final MemorySegment[] map(
}
i
jpountz commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535342453
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -54,14 +60,16 @@ public enum Context {
public static final IOContext DEFAULT = new IOContext
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535331259
##
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##
@@ -92,10 +110,22 @@ private final MemorySegment[] map(
}
jpountz commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535326207
##
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##
@@ -92,10 +110,22 @@ private final MemorySegment[] map(
}
if (
jpountz commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535324025
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -54,14 +60,16 @@ public enum Context {
public static final IOContext DEFAULT = new IOContext
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535309204
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -54,14 +60,16 @@ public enum Context {
public static final IOContext DEFAULT = new IOCo
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535291132
##
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##
@@ -92,10 +110,22 @@ private final MemorySegment[] map(
}
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535283798
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one o
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535283242
##
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##
@@ -92,10 +110,22 @@ private final MemorySegment[] map(
}
i
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535276508
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535274772
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535248163
##
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##
@@ -92,10 +110,22 @@ private final MemorySegment[] map(
}
i
jpountz commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535211651
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -54,14 +60,16 @@ public enum Context {
public static final IOContext DEFAULT = new IOContext
jpountz commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1535211651
##
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##
@@ -54,14 +60,16 @@ public enum Context {
public static final IOContext DEFAULT = new IOContext
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1534775594
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one o
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1534727218
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1534678873
##
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##
@@ -92,10 +110,19 @@ private final MemorySegment[] map(
}
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1534636363
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one o
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1534598790
##
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##
@@ -92,10 +110,19 @@ private final MemorySegment[] map(
}
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2013499830
Unfortunately after the problems with other constants I tend to hide the
advice constants in the class any use some abstraction, so we can call
We should maybe also simply remov
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2013453556
I removed the page size retrieval again. No need for it, we just be safe and
only apply the advice, when the chunk size is large enough.
--
This is an automated message from the Apa
uschindler commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2013348340
I refactored the code a bit:
- @jpountz I added the merge context (I checked DirectIODirectory).
- I retrieve the pageSize to check if a segment is correctly aligned.
Unfortunate
uschindler commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1534481239
##
lucene/core/src/java21/org/apache/lucene/store/PosixNativeAccess.java:
##
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
ChrisHegarty commented on code in PR #13196:
URL: https://github.com/apache/lucene/pull/13196#discussion_r1534254114
##
lucene/core/src/java21/org/apache/lucene/store/NativeAccess.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or mor
jpountz commented on PR #13196:
URL: https://github.com/apache/lucene/pull/13196#issuecomment-2012967809
I remember this kind of things being discussed more than 10 years ago, it's
extremely exciting to see it close to being included in the default `Directory`!
> current "readOnce =>
uschindler opened a new pull request, #13196:
URL: https://github.com/apache/lucene/pull/13196
This is a first idea how we can use Panama Foreign to pass `madvise()` hints
to the kernel when mapping memory segments.
The code looks up the function pointer from stdlib (libc) on Linux an
67 matches
Mail list logo
