salvatore-campagna commented on code in PR #16623:
URL: https://github.com/apache/lucene/pull/16623#discussion_r4295256879


##########
lucene/core/src/java/org/apache/lucene/util/FixedBitSet.java:
##########
@@ -871,9 +871,15 @@ public static FixedBitSet copyOf(Bits bits) {
       int length = bits.length();
       FixedBitSet bitSet = new FixedBitSet(length);
       bitSet.set(0, length);
-      for (int i = 0; i < length; ++i) {
-        if (bits.get(i) == false) {
-          bitSet.clear(i);
+      if (bits instanceof LiveDocs) {

Review Comment:
   This assumes every `LiveDocs` overrides `applyMask` with a bulk 
implementation, but `LiveDocs` does not document that expectation. 
`AssertingLeafReader.AssertingLiveDocs` does not override it, so it takes the 
default, which the comment below calls several times slower than the loop. That 
only affects tests today.
   
   Nit: should `AssertingLiveDocs` override `applyMask` and keep its thread 
check, and should `LiveDocs` say implementations are expected to override it? 
The wrapper part could be a follow up.



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

Reply via email to