jmazanec15 commented on code in PR #12050:
URL: https://github.com/apache/lucene/pull/12050#discussion_r1062966074


##########
lucene/core/src/java/org/apache/lucene/util/hnsw/OnHeapHnswGraph.java:
##########
@@ -94,36 +93,83 @@ public int size() {
   }
 
   /**
-   * Add node on the given level
+   * Add node on the given level. Nodes can be inserted out of order, but it 
requires that the nodes

Review Comment:
   > but still because in L156 we need to copy the rest of array again and 
again as long as that is a non-appending action
   
   Right, this could be expensive for out of order insertion. I can try 
switching the nodeByLevel int array to a TreeSet and compare performance to 
https://github.com/apache/lucene/issues/11354.
   
   One complication with this approach is that the NodesIterator expects an int 
array: 
https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/util/hnsw/HnswGraph.java#L134.
 Given this is a public interface, we might need to either convert the treeset 
to an int array every time 
[getNodesOnLevel](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/util/hnsw/OnHeapHnswGraph.java#L165)
 gets called, or alter the NodesIterator interface to support both an int array 
and an Iterator produced from the TreeSet.
   
   @zhaih What do you think of this approach? Is there better way to do this?



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