Remove redundant code in Lucene search

[ErikPelli]

Weight: Move the twoPhase argument check in the loop, so the condition is true when twoPhase argument is null or, when not null, if matches() return true.

IndexSearcher: group is an ArrayList inserted by reference into groupedLeaves, so we can simply add to group outside of the if(group==null) check and keep it to initialize the group.

[ErikPelli]

The change to IndexSearcher#slices looks fine. I'm less sure about changes to DefaultBulkScorer: it puts more pressure on the compiler to detect that twoPhase == null is a loop invariant, we've been undoing the sort of change that you're doing here in the past in order to get better performance. Would you be able to run a benchmark to confirm that this change doesn't hurt search performance?

Effectively I compared the two compiled bytecodes and doesn't seem to detect this optimization, but maybe at runtime the jvm does something for that or maybe an if check is negligible for the performance loss (depends how many times it's called by the way), a benchmark would be useful to discover that, do you know if there is already one for that or if it needs to be written from scratch? I searched for more information about this change in the history and apparently this part of code was introduced in 2015 (https://github.com/apache/lucene/commit/d671dd8d890a8e5eb56cbcd94873c3057745a17f from you, you've been here a long time I see) but the precedent code didn't contain an if inside the loop so it would be interesting to see if there is an effective change in performance in search from this pull request or if it doesn't affect anything and a more elegant code can be applied.

[github-actions[bot]]

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the [email protected] list. Thank you for your contribution!

[jpountz]

Sorry for the very long delay. This sort of detection of loop invariants is indeed handled by C1/C2 compilers, not javac. Without a benchmark suggesting that there is no performance impact, I would err on the safe side and keep the check outside of the loop.

[github-actions[bot]]

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the [email protected] list. Thank you for your contribution!