mikemccand commented on code in PR #12738:
URL: https://github.com/apache/lucene/pull/12738#discussion_r1381564163


##########
lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java:
##########
@@ -110,25 +117,39 @@ public long add(FSTCompiler.UnCompiledNode<T> nodeIn) 
throws IOException {
         node = getFallback(nodeIn, hash);
         if (node != 0) {
           // it was already in fallback -- promote to primary
-          primaryTable.set(pos, node);
+          // TODO: Copy directly between 2 ByteBlockPool to avoid double-copy
+          primaryTable.set(pos, node, fallbackTable.getBytes(pos, 
lastFallbackNodeLength));
         } else {
           // not in fallback either -- freeze & add the incoming node
 
+          long startAddress = fstCompiler.bytes.getPosition();
           // freeze & add
           node = fstCompiler.addNode(nodeIn);
 
+          // TODO: Write the bytes directly from BytesStore
           // we use 0 as empty marker in hash table, so it better be 
impossible to get a frozen node
           // at 0:
-          assert node != 0;
+          assert node != FST.FINAL_END_NODE && node != FST.NON_FINAL_END_NODE;
+          byte[] buf = new byte[Math.toIntExact(node - startAddress + 1)];

Review Comment:
   Brain hurts!  Reverse ob1 error in my brain :)
   
   What confuses me here is if `fstCompiler.addNode` writes 3 bytes, won't we 
compute a length of 4?
   
   Oh, I see!  `FSTCompiler#addNode` has this at the end:
   
   ```
       final long thisNodeAddress = bytes.getPosition() - 1;
       bytes.reverse(startAddress, thisNodeAddress);
       nodeCount++;
       return thisNodeAddress;
   ```
   
   So that last byte address it returns is inclusive, since it did the `-1`, so 
we have to +1 to undo that.  OK I think it makese sense ;)
   



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