gsmiller commented on issue #15905:
URL: https://github.com/apache/lucene/issues/15905#issuecomment-4185954654

   With-respect-to `ScoreDoc[]` vs. `int[]`, I agree with what you're saying 
about Option 1 vs. Option 3. Ultimately, I'm slightly concerned we're going to 
end up with four methods proposed by the end of this that model the different 
combinations of ScoreDoc[] and int[] as input across ordinals or no-ordinals 
:). Maybe it was a mistake to move from int[] to ScoreDoc[] in the previous API 
design. If int[] really is the more generic version of this, maybe the right 
thing is to ask callers to deal with translating their ScoreDoc[] to int[]. I 
think the earlier premise was that the common use-case was for callers to have 
ScoreDoc[], so it made sense to not expect them to translate. But stepping back 
a little, maybe we should consider this decision separately?
   
   Just to make things complicated though (sorry!), did you consider an 
alternative approach that would just return ordinals as int[][]? So instead of 
sorting the actual ScoreDoc[] or int[] input, we'd provide ordinal references 
back into the original data. Maybe an example would be easier:
   ```
   ScoreDoc[] hits = {new ScoreDoc(42, ..), new ScoreDoc(7, ..), new 
ScoreDoc(100, ..), new ScoreDoc(23, ..)};
   
   int[][] ordinalsByLeaf = ReaderUtil.partitionByLeaf(hits, leaves);
   // Suppose leaf 0 covers docs 0-49, leaf 1 covers docs 50-99, leaf 2 covers 
100+
   // ordinalsByLeaf[0] = {1, 3, 0}  → hits[1]=doc7, hits[3]=doc23, 
hits[0]=doc42 (sorted within leaf)
   // ordinalsByLeaf[1] = {}
   // ordinalsByLeaf[2] = {2}        → hits[2]=doc100
   ```
   
   So this handles both the current use-case and the scatter-gather use-case I 
think in a single method? (Different topic on whether-or-not you need two 
versions for accepting ScoreDoc[] vs. accepting int[]). I think this will still 
underperform similarly as Option 3 based on the benchmarking I did earlier 
though. You still couldn't use Arrays#sort directly; you'd need need something 
like IntroSorter, and sorting would have to do two array dereferences to access 
each element for comparison (i.e., `hits[ordinalsByLeaf[i]]`). But maybe it 
avoids some data copying? It might be worth adding to the jmh benchmark I 
created just to see...


-- 
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]

Reply via email to