uschindler commented on code in PR #13907:
URL: https://github.com/apache/lucene/pull/13907#discussion_r1801030063


##########
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInput.java:
##########
@@ -567,7 +567,8 @@ public final MemorySegmentIndexInput slice(String 
sliceDescription, long offset,
   public final MemorySegmentIndexInput slice(
       String sliceDescription, long offset, long length, ReadAdvice advice) 
throws IOException {
     MemorySegmentIndexInput slice = slice(sliceDescription, offset, length);
-    if (NATIVE_ACCESS.isPresent()) {
+    if (NATIVE_ACCESS.isPresent() && advice != ReadAdvice.NORMAL) {

Review Comment:
   At moment the slice method would not allow to reset the read advice for a 
slice back to NORMAL. This is perfectly fine here, because this methods is only 
used for CFS files (which are always opened with NORMAL).
   
   We should just keep this in mind. The Javadocs ob IndexInput already 
mentions why this method is available and should be used only for compound 
files. I just wanted to mention this here for the record, no need to fix 
anything.
   
   Another idea for discussion possibly in another PR: How about to save the 
advice/IOContext that was used for opening the input as field in 
MemorySegmentIndexInput and only set the advise here, when it differs from the 
current?



##########
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInputProvider.java:
##########
@@ -129,7 +129,9 @@ private final MemorySegment[] map(
       // internal FileChannel logic)
       if (preload) {
         segment.load();
-      } else if (nativeAccess.filter(na -> segment.address() % 
na.getPageSize() == 0).isPresent()) {
+      } else if (readAdvice != ReadAdvice.NORMAL

Review Comment:
   Idea for another PR: When we extend the whole code to be able to change read 
advice later, we could move this code to the MemorySegmentIndexInput class. 
Wecould just pass the advise or IOContext to the instance created (see above 
for more info).



-- 
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: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to