ErikPelli commented on PR #12035: URL: https://github.com/apache/lucene/pull/12035#issuecomment-1365878989
> 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. -- 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
