gsmiller commented on code in PR #16496:
URL: https://github.com/apache/lucene/pull/16496#discussion_r3770752122
##########
lucene/core/src/java/org/apache/lucene/index/ReaderUtil.java:
##########
@@ -95,25 +95,122 @@ public static int subIndex(int n, List<LeafReaderContext>
leaves) {
}
/**
- * Partitions global doc IDs from ScoreDoc array by leaf. Extracts doc IDs,
sorts them, and
- * partitions across leaves.
+ * Partitions global doc IDs by leaf. Doc IDs may be supplied in any order;
the returned per-leaf
+ * arrays are sorted in ascending docId order.
*
- * @param hits the ScoreDoc array (typically from TopDocs.scoreDocs)
+ * <p>This is an optimized subset of {@link
#partitionByLeafWithOrdinals(int[], List)} for callers
+ * that only need the per-leaf grouping and do not need to map results back
to the original input
+ * order. It sorts with {@link Arrays#sort(int[])} and skips the extra
bookkeeping required to
+ * track input ordinals. Callers that need to reassemble per-leaf results
into input order
+ * (scatter/gather) should use {@link #partitionByLeafWithOrdinals(int[],
List)} instead.
+ *
+ * <p>The input array is not mutated.
+ *
+ * @param globalDocIds global doc IDs in any order
* @param leaves the index reader's leaves
- * @return array indexed by leaf ord, containing global doc IDs for that
leaf (empty if no hits)
+ * @return array indexed by leaf ord, containing the (sorted) global doc IDs
for that leaf (empty
+ * if no hits land in that leaf)
*/
- public static int[][] partitionByLeaf(ScoreDoc[] hits,
List<LeafReaderContext> leaves) {
+ public static int[][] partitionByLeaf(int[] globalDocIds,
List<LeafReaderContext> leaves) {
int numLeaves = leaves.size();
- int[][] result = new int[numLeaves][];
- if (hits.length == 0) {
+ if (globalDocIds.length == 0) {
+ int[][] result = new int[numLeaves][];
Arrays.fill(result, EMPTY_INT_ARRAY);
return result;
}
- int[] sortedDocIds = new int[hits.length];
- for (int i = 0; i < hits.length; i++) {
- sortedDocIds[i] = hits[i].doc;
- }
+ int[] sortedDocIds = globalDocIds.clone();
Arrays.sort(sortedDocIds);
+ return partitionSortedDocIds(sortedDocIds, leaves);
+ }
+
+ /**
+ * Result of partitioning doc IDs by leaf, including the original input
ordinals for
+ * scatter/gather. {@code docIdsByLeaf[k]} holds the sorted global doc IDs
that fall in leaf
+ * {@code k}, and {@code ordinalsByLeaf[k][i]} is the index in the original
{@code globalDocIds}
+ * input array of the doc ID at {@code docIdsByLeaf[k][i]}.
+ *
+ * <p>Both arrays have the same shape: {@code ordinalsByLeaf[k].length ==
docIdsByLeaf[k].length}
+ * for every leaf {@code k}.
+ *
+ * @param docIdsByLeaf per-leaf sorted global doc IDs; {@code
docIdsByLeaf[k]} holds the doc IDs
+ * that fall in leaf {@code k} (empty if none)
+ * @param ordinalsByLeaf per-leaf original input positions; {@code
ordinalsByLeaf[k][i]} is the
+ * index in the original input array of the doc ID at {@code
docIdsByLeaf[k][i]}
+ */
+ public record PartitionedHits(int[][] docIdsByLeaf, int[][] ordinalsByLeaf)
{}
+
+ /**
+ * Partitions global doc IDs by leaf, tracking each doc ID's original
position in the input array
+ * so callers can reassemble per-leaf results back to input order
(scatter/gather).
+ *
+ * <p>This is the fuller-featured counterpart to {@link
#partitionByLeaf(int[], List)}: it returns
+ * the same per-leaf grouping and additionally records, for every
partitioned doc ID, its index in
+ * the original {@code globalDocIds} array. Tracking these ordinals carries
a small amount of
+ * extra work relative to {@link #partitionByLeaf(int[], List)}, so callers
that do not need to
+ * map results back to input order should prefer that method.
+ *
+ * <p>The input array is not mutated.
+ *
+ * @param globalDocIds global doc IDs in any order (e.g., ranking order)
+ * @param leaves the index reader's leaves
+ * @return per-leaf sorted doc IDs alongside per-leaf ordinals into the
input array
+ */
+ public static PartitionedHits partitionByLeafWithOrdinals(
+ int[] globalDocIds, List<LeafReaderContext> leaves) {
+ int numLeaves = leaves.size();
+ if (globalDocIds.length == 0) {
+ int[][] docIdsByLeaf = new int[numLeaves][];
+ int[][] ordinalsByLeaf = new int[numLeaves][];
+ Arrays.fill(docIdsByLeaf, EMPTY_INT_ARRAY);
+ Arrays.fill(ordinalsByLeaf, EMPTY_INT_ARRAY);
+ return new PartitionedHits(docIdsByLeaf, ordinalsByLeaf);
+ }
+
+ // Sort doc IDs and ordinals as parallel arrays, so we keep the original
positions while
+ // moving doc IDs into ascending order. IntroSorter avoids the
boxing/lambda overhead a
+ // comparator-based Arrays.sort would incur on parallel int[]s.
+ final int[] sortedDocIds = globalDocIds.clone();
+ final int[] sortedOrdinals = new int[globalDocIds.length];
+ for (int i = 0; i < sortedOrdinals.length; i++) {
+ sortedOrdinals[i] = i;
+ }
+ new IntroSorter() {
Review Comment:
Cool, just pushed those changes (should only have touched the benchmark
code, not any of your actual code). Also posting the results I got from running
the benchmark here. It's really interesting (to me at least) to see the
benchmark validate the merit of the packed-long approach vs. intro-sort. It's
also interesting to see how much it closes the perf gap to not doing any
ordinal tracking. I'd be curious what you think of keeping both APIs vs.
consolidating into one (essentially not worrying about the performance penalty
of ordinal tracking for cases where ordinals aren't needed in favor of one
API). Also please feel free to run the benchmarks and see if you're getting
similar results, forming different conclusions, etc., etc. :) Thanks!
## Raw results
| numDocIds | numLeaves | partitionByLeaf (plain) |
partitionByLeafWithOrdinals (packed) | …IntroSort |
|---:|---:|---:|---:|---:|
| 100 | 10 | 1486.7 ± 243.8 | 1188.3 ± 81.3 | 1134.1 ± 63.7 |
| 100 | 200 | 594.1 ± 49.9 | 570.3 ± 8.4 | 433.5 ± 8.1 |
| 1000 | 10 | 89.52 ± 41.50 | 92.03 ± 10.96 | 69.28 ± 2.13 |
| 1000 | 200 | 73.71 ± 7.86 | 65.14 ± 9.20 | 51.59 ± 2.21 |
| 10000 | 10 | 5.095 ± 1.321 | 5.885 ± 0.986 | 2.750 ± 0.321 |
| 10000 | 200 | 5.545 ± 2.494 | 5.916 ± 0.499 | 3.194 ± 0.587 |
| 100000 | 10 | 0.242 ± 0.015 | 0.243 ± 0.010 | 0.168 ± 0.006 |
| 100000 | 200 | 0.251 ± 0.007 | 0.245 ± 0.004 | 0.167 ± 0.006 |
## Packed-long vs. IntroSort
| numDocIds | numLeaves | Packed speedup |
|---:|---:|---:|
| 100 | 10 | 1.05× |
| 100 | 200 | 1.32× |
| 1000 | 10 | 1.33× |
| 1000 | 200 | 1.26× |
| 10000 | 10 | 2.14× |
| 10000 | 200 | 1.85× |
| 100000 | 10 | 1.45× |
| 100000 | 200 | 1.47× |
## Ordinal overhead vs the plain no-ordinals API
(Overhead = (plain − variant) ÷ plain)
| numDocIds | numLeaves | Packed overhead | IntroSort overhead |
|---:|---:|---:|---:|
| 100 | 10 | +20.1% | +23.7% |
| 100 | 200 | +4.0% | +27.0% |
| 1000 | 10 | -2.8% | +22.6% |
| 1000 | 200 | +11.6% | +30.0% |
| 10000 | 10 | -15.5% | +46.0% |
| 10000 | 200 | -6.7% | +42.4% |
| 100000 | 10 | -0.4% | +30.6% |
| 100000 | 200 | +2.4% | +33.5% |
## Raw JMH output
```
Benchmark (numDocIds)
(numLeaves) Mode Cnt Score Error Units
PartitionByLeafBenchmark.partitionByLeaf 100
10 thrpt 8 1486.740 ± 243.770 ops/ms
PartitionByLeafBenchmark.partitionByLeaf 100
200 thrpt 8 594.129 ± 49.858 ops/ms
PartitionByLeafBenchmark.partitionByLeaf 1000
10 thrpt 8 89.516 ± 41.496 ops/ms
PartitionByLeafBenchmark.partitionByLeaf 1000
200 thrpt 8 73.707 ± 7.861 ops/ms
PartitionByLeafBenchmark.partitionByLeaf 10000
10 thrpt 8 5.095 ± 1.321 ops/ms
PartitionByLeafBenchmark.partitionByLeaf 10000
200 thrpt 8 5.545 ± 2.494 ops/ms
PartitionByLeafBenchmark.partitionByLeaf 100000
10 thrpt 8 0.242 ± 0.015 ops/ms
PartitionByLeafBenchmark.partitionByLeaf 100000
200 thrpt 8 0.251 ± 0.007 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinals 100
10 thrpt 8 1188.346 ± 81.269 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinals 100
200 thrpt 8 570.284 ± 8.370 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinals 1000
10 thrpt 8 92.032 ± 10.960 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinals 1000
200 thrpt 8 65.144 ± 9.195 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinals 10000
10 thrpt 8 5.885 ± 0.986 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinals 10000
200 thrpt 8 5.916 ± 0.499 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinals 100000
10 thrpt 8 0.243 ± 0.010 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinals 100000
200 thrpt 8 0.245 ± 0.004 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinalsIntroSort 100
10 thrpt 8 1134.130 ± 63.681 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinalsIntroSort 100
200 thrpt 8 433.474 ± 8.146 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinalsIntroSort 1000
10 thrpt 8 69.280 ± 2.132 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinalsIntroSort 1000
200 thrpt 8 51.595 ± 2.211 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinalsIntroSort 10000
10 thrpt 8 2.750 ± 0.321 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinalsIntroSort 10000
200 thrpt 8 3.194 ± 0.587 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinalsIntroSort 100000
10 thrpt 8 0.168 ± 0.006 ops/ms
PartitionByLeafBenchmark.partitionByLeafWithOrdinalsIntroSort 100000
200 thrpt 8 0.167 ± 0.006 ops/ms
```
--
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]