dweiss commented on code in PR #13936:
URL: https://github.com/apache/lucene/pull/13936#discussion_r1807927256
##########
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:
> Not quite, the idea was that I already have heap in a local in the caller,
so if I pass it as an argument I save a field read and as an added bonus get a
smaller method that inlines better.
I did understand the intention but I think the difference, if any, will be
noticeable only if the loop doesn't hoist out the field read (which, I think it
should?). My suggestion keeps the variables local, which helps in understanding
of what it does. But anyway. I'm not entirely sold on these low-level
optimizations that target c2/hotspot. There is so many moving parts here...
operating system and CPU included. Eh.
> 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? :)
I believe the differences were also functional - insertWithOverflow is one
particular example that comes to mind and would require more complex logic in
the JDK's PQ. Another is resigning from one level of indirection (method
instead of Comparator<T>) - these choices predate a lot of newer Java's
offerings - perhaps it could be implemented in a different way now.
--
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
