You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "jpountz (via GitHub)" <gi...@apache.org> on 2024/04/04 17:51:21 UTC

[PR] Use IOContext#RANDOM when appropriate. [lucene]

jpountz opened a new pull request, #13267:
URL: https://github.com/apache/lucene/pull/13267

   This switches the following files to `IOContext#RANDOM`:
    - Stored fields data file.
    - Term vectors data file.
    - HNSW graph.
    - Temporary file storing vectors at merge time that we use to construct the merged HNSW graph.
    - Vector data files, including quantized data files.
   
   I hesitated using `IOContext.RANDOM` on terms, since they have a random access pattern when running term queries, but a more sequential access pattern when running multi-term queries. I erred on the conservative side and did not switch them to `IOContext.RANDOM` for now.
   
   For simplicity, I'm only touching the current codec, not previous codecs.


-- 
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-unsubscribe@lucene.apache.org

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


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

Re: [PR] Use IOContext#RANDOM when appropriate. [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13267:
URL: https://github.com/apache/lucene/pull/13267#discussion_r1552487587


##########
lucene/CHANGES.txt:
##########
@@ -78,6 +78,12 @@ Optimizations
 * GITHUB#13149: Made PointRangeQuery faster, for some segment sizes, by reducing the amount of virtual calls to
   IntersectVisitor::visit(int). (Anton Hägerstrand)
 
+* GITHUB#13222: Vector data files - including quantized vectors, HNSW graphs,
+  stored fields data files and term vectors data files now advise the
+  underlying `Directory` to optimize for random-access. On recent Java versions
+  (>= 21) and POSIX systems, `MMapDirectory` would use MADV_RANDOM for these

Review Comment:
   It is a bit unclear: both conditions must apply. "On Linux and macOS systems using recent Java versions (>=21), MMapDirectory would use POSIX_MADV_RANDOM"



-- 
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-unsubscribe@lucene.apache.org

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


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

Re: [PR] Use IOContext#RANDOM when appropriate. [lucene]

Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz merged PR #13267:
URL: https://github.com/apache/lucene/pull/13267


-- 
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-unsubscribe@lucene.apache.org

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


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

Re: [PR] Use IOContext#RANDOM when appropriate. [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13267:
URL: https://github.com/apache/lucene/pull/13267#discussion_r1552492150


##########
lucene/CHANGES.txt:
##########
@@ -78,6 +78,12 @@ Optimizations
 * GITHUB#13149: Made PointRangeQuery faster, for some segment sizes, by reducing the amount of virtual calls to
   IntersectVisitor::visit(int). (Anton Hägerstrand)
 
+* GITHUB#13222: Vector data files - including quantized vectors, HNSW graphs,
+  stored fields data files and term vectors data files now advise the
+  underlying `Directory` to optimize for random-access. On recent Java versions
+  (>= 21) and POSIX systems, `MMapDirectory` would use MADV_RANDOM for these

Review Comment:
   Maybe combine this with above entry about madvise.



-- 
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-unsubscribe@lucene.apache.org

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


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

Re: [PR] Use IOContext#RANDOM when appropriate. [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #13267:
URL: https://github.com/apache/lucene/pull/13267#discussion_r1552390907


##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99FlatVectorsWriter.java:
##########
@@ -315,10 +316,13 @@ public CloseableRandomVectorScorerSupplier mergeOneFieldToIndex(
       CodecUtil.writeFooter(tempVectorData);
       IOUtils.close(tempVectorData);
 
-      // copy the temporary file vectors to the actual data file
+      // This temp file will be accessed in a random-access fashion to construct the HNSW graph.
+      // Note: don't use the context from the state, which is a flush/merge context, not expecting
+      // to perform random reads.
       vectorDataInput =
           segmentWriteState.directory.openInput(
-              tempVectorData.getName(), segmentWriteState.context);
+              tempVectorData.getName(), IOContext.DEFAULT.withRandomAccess());

Review Comment:
   Here you could also use `IOContext.RANDOM` directly.



##########
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##########
@@ -132,10 +132,24 @@ public IOContext(IOContext ctxt, boolean readOnce) {
     this.mergeInfo = ctxt.mergeInfo;
     this.flushInfo = ctxt.flushInfo;
     this.readOnce = readOnce;
-    this.randomAccess = ctxt.randomAccess;
+    this.randomAccess = false;

Review Comment:
   I think this should be documented.



-- 
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-unsubscribe@lucene.apache.org

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


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