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