original-brownbear commented on code in PR #13936:
URL: https://github.com/apache/lucene/pull/13936#discussion_r1807876659


##########
lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java:
##########
@@ -270,7 +280,7 @@ public final boolean remove(T element) {
     return false;
   }
 
-  private final boolean upHeap(int origPos) {
+  private boolean upHeap(int origPos, T[] heap) {

Review Comment:
   > >long-term maintenance as worth the tiny performance benefit
   
   With this class in particular I'm not sure the argument holds. Isn't the 
whole point of it the ability to mutate top and resort via `updateTop` as an 
optimization over the JDKs priority queue? If the implementation is slower than 
java.util.PriorityQueue, then what's the point? :) Also, I'm still not sure I 
agree with the "tiny" part :) 
   Granted there's limits to the benchmark data provided, but it's more likely 
than not that a couple things improved by 3%+ isn't it? Plus, I could see a 
possible compounding effect with further optimizations in the users of the PQ 
if those can be reduced in size enough to have `lessThan` inline and not be a 
megamorphic callsite here and there.



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