gsmiller commented on code in PR #12812:
URL: https://github.com/apache/lucene/pull/12812#discussion_r1420739649


##########
lucene/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java:
##########
@@ -179,6 +179,19 @@ static class SlicedIntBlockPool extends IntBlockPool {
       super(allocator);
     }
 
+    /**
+     * For slices, buffers must be filled with zeros, so that we can find a 
slice's end based on a
+     * non-zero final value.
+     */
+    private static boolean assertSliceBuffer(int[] buffer) {
+      for (int value : buffer) {

Review Comment:
   Since it's only used in asserts, I think readability is probably 
preferential to performance? The way you have it here in this change would be 
the most readable IMO, and probably makes the most sense. Let's keep the change 
the way you have it but maybe just update the CHANGES entry to highlight that 
you're making the assertion defensive against overflow and negative values? 
That might leave a better "digital archeology" trail for the future so someone 
doesn't make the same mistake I did, read this change as an optimization and 
then go off wondering if a branchless implementation wouldn't make more 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