rmuir commented on a change in pull request #315:
URL: https://github.com/apache/lucene/pull/315#discussion_r721296208



##########
File path: 
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90HnswVectorsReader.java
##########
@@ -301,55 +349,62 @@ public KnnGraphValues getGraphValues(String field) throws 
IOException {
       throw new IllegalArgumentException("No such field '" + field + "'");
     }
     FieldEntry entry = fields.get(field);
-    if (entry != null && entry.indexDataLength > 0) {
+    if (entry != null && entry.graphIndexLength > 0) {
       return getGraphValues(entry);
     } else {
       return KnnGraphValues.EMPTY;
     }
   }
 
   private KnnGraphValues getGraphValues(FieldEntry entry) throws IOException {
+    if (entry.ordOffsetsByLevel == null) {
+      synchronized (entry) {

Review comment:
       can we remove this? Looks like lazy-loading.
   
   The problem with this, is that it doesn't buy anything to lazy-load. At 
first it might seem like it saves some memory if the feature isn't used, until 
merge happens, then BOOM.
   
   I think instead we should keep it simple and just load up front, and spend 
our time trying to reduce the amount of memory that this thing uses.




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