benwtrent commented on code in PR #14426:
URL: https://github.com/apache/lucene/pull/14426#discussion_r2042590532


##########
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsReader.java:
##########
@@ -130,4 +134,48 @@ public KnnVectorsReader getMergeInstance() {
    * <p>The default implementation is empty
    */
   public void finishMerge() throws IOException {}
+
+  /**
+   * Returns the desired size of off-heap memory the given field. This size 
can be used to help
+   * determine the memory requirements for optimal search performance, which 
can be greatly affected
+   * by page faults when not enough memory is available.
+   *
+   * <p>For reporting purposes, the size of the off-heap index structures is 
broken down by their
+   * file extension, which provides a logical categorization of their purpose, 
e.g. the {@code
+   * Lucene99HnswVectorsFormat} stores the HNSW graph neighbours lists in a 
file with the "vex"
+   * extension.
+   *
+   * <p>The long value is the size in bytes of the off-heap space needed if 
the associated index
+   * structure were to be fully loaded in memory. While somewhat analogous to 
{@link
+   * Accountable#ramBytesUsed()} (which reports actual on-heap memory usage), 
the sizes reported by
+   * this method are not actual usage but rather the amount of available 
memory needed to fully load
+   * the index into memory, rather than an actual RAM usage requirement.
+   *
+   * <p>To determine the total desired off-heap memory size for the given 
field:
+   *
+   * <pre>{@code
+   * 
getOffHeapByteSize(field).values().stream().mapToLong(Long::longValue).sum();
+   * }</pre>
+   *
+   * @param fieldInfo the fieldInfo
+   * @return a map of the desired off-heap memory requirements by category
+   * @lucene.experimental
+   */
+  public abstract Map<String, Long> getOffHeapByteSize(FieldInfo fieldInfo);

Review Comment:
   Doesn't this need to be backwards compatible? Here we are adding a new 
required base function to be overridden by all extenders of the 
`KnnVectorsReader`.
   
   Marking it as experimental is good. But maybe it supplies a default 
implementation that simply returns `null` or `Map.of()`.



-- 
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