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