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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
