gsmiller commented on issue #15905: URL: https://github.com/apache/lucene/issues/15905#issuecomment-4183884261
I have a few thoughts but I'm more curious what others think. As a disclaimer I work with Zihan on Amazon's product search, but I'm not particularly close to this project (so I'll probably ask silly questions) :) 1. Between the first two options, I think two separate methods is the way to go (option 1). Adding a boolean to control behavior (option 2) has a bit of a bad smell IMO. 2. Is there a reason you want the newly-proposed `partitionByLeafWithOrdinals` method to take `int[]` instead of `ScoreDoc[]` as input (in option 1)? There's a bit of asymmetry that might be worth fleshing out. It feels like these two methods are conflating whether-or-not the caller has ScoreDoc[] or int[] as input _and_ whether-or-not they need "ordinal tracking." This sort of carries into option 3 too. If you went with that approach, would you use ScoreDoc[] as input (as the current API does), or would you need to change it to int[]? 3. Would option 3 be as simple as leveraging [IntroSorter](https://github.com/apache/lucene/blob/0fcf9c825f1f76588040f592d7f893899cc715cd/lucene/core/src/java/org/apache/lucene/util/IntroSorter.java#L39) to sort the docIDs and ordinals as parallel arrays [1]? I wonder how much overhead it would actually introduce to do this over the current Arrays#sort approach. I kind of think we should try this first and see if it actually results in a meaningful performance difference. So I think I'm in favor of starting with option 3. If there really is a meaningful performance impact, I'd be in favor of option 1 (but sorting out the discussion of ScoreDoc[] vs. int[] as input). [1] Something like... ``` new IntroSorter() { int pivot; @Override protected int compare(int i, int j) { return Integer.compare(sortedDocIds[i], sortedDocIds[j]); } @Override protected void swap(int i, int j) { int tmp = sortedDocIds[i]; sortedDocIds[i] = sortedDocIds[j]; sortedDocIds[j] = tmp; tmp = ordinals[i]; ordinals[i] = ordinals[j]; ordinals[j] = tmp; } @Override protected void setPivot(int i) { pivot = sortedDocIds[i]; } @Override protected int comparePivot(int j) { return Integer.compare(pivot, sortedDocIds[j]); } }.sort(0, sortedDocIds.length); ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
