Skip to content

Conversation

dweiss
Copy link
Contributor

@dweiss dweiss commented Oct 8, 2025

No description provided.

@dweiss dweiss linked an issue Oct 8, 2025 that may be closed by this pull request
@dweiss dweiss added this to the 11.0.0 milestone Oct 8, 2025
Comment on lines +222 to +225
// Lucene's PriorityQueue.remove uses reference equality, not .equals to determine which
// elements
// to remove (!).
HashMap<Integer, Integer> ints = new HashMap<>();
Copy link
Contributor Author

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?).

Copy link
Member

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rmuir
Copy link
Member

rmuir commented Oct 8, 2025

I'll help with merging this branch out of conflicts: I'm not actually "fixing" anything in main, just @Nightly'ing the long tail as a cortisone shot. I probably cause some conflicts which will be easy to resolve.

@dweiss
Copy link
Contributor Author

dweiss commented Oct 8, 2025

No worries, I'll handle the merge and conflicts.

@dweiss dweiss marked this pull request as ready for review October 9, 2025 20:09
@dweiss
Copy link
Contributor Author

dweiss commented Oct 9, 2025

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.

@dweiss dweiss merged commit 6a1ff24 into main Oct 9, 2025
12 checks passed
@dweiss dweiss deleted the 15212-speed-up-slow-tests branch October 9, 2025 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Speed up slow tests

2 participants