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