-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Speed up slow tests (#15212) #15306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Speed up slow tests (#15212) #15306
Conversation
…st java's priority queue. Same net result.
// Lucene's PriorityQueue.remove uses reference equality, not .equals to determine which | ||
// elements | ||
// to remove (!). | ||
HashMap<Integer, Integer> ints = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test had a huge iteration count, was checking tons of stuff unnecessarily... I rewrote it to just compare against Java's PriorityQueue... but it turned out that lucene's PriorityQueue.remove(T) is implemented as a linear scan through all elements, with reference equality (which is surprising to say the least...). That's why this "cache" of ints needs to be here.
PQ.remove is only used from one place within Lucene... maybe we should drop this method entirely (or at least change == to .equals to be less surprising?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to open a followup issue on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll help with merging this branch out of conflicts: I'm not actually "fixing" anything in |
No worries, I'll handle the merge and conflicts. |
I'll merge this in. There's been work on speeding up tests on other branches and indeed - we can just tackle them individually, on need for this branch. |
No description provided.