rishabhmaurya commented on PR #13186: URL: https://github.com/apache/lucene/pull/13186#issuecomment-2010431193
> Your reasoning about why the swap logic is inefficient seems to assume that there would be lots of equal elements, but actually since we tie break on docID, there would never be any equal elements and phase 2 is a no-op in practice? yes! it slipped my head that we tie break on docID. You're right, phase-2 here is a no-op here. However, do you see issue with recomputing the bias on [swap(i, j)](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/util/IntroSelector.java#L110) in phase-1. It probably works since we do `++i` and `--j` in next iteration and never use `i` or `j` ever again while comparison, so its probably harmless to change their bias. > I have a bias against this because it would further increase RAM requirements by 4 bytes (one integer) per doc ID? or a boolean per doc, if it moved across middle as that's we recompute. > If we see evidence that swapping is a bottleneck, I'd be more keen to looking into using something like RadixSelector, which should perform less useless swapping. That makes sense. I have some evidences with `http_logs` workload (https://github.com/apache/lucene/pull/13123#discussion_r1513889998) but again it may not be the best workload to benchmark this feature. I will run benchmark against other workloads too and share the numbers here, then we can see if swapping is a bottleneck and make a call. Thanks for reviewing and implementing the reorderer. -- 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
