slow-J commented on code in PR #16268:
URL: https://github.com/apache/lucene/pull/16268#discussion_r3482200089
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java:
##########
@@ -275,6 +363,40 @@ public boolean advanceExact(int doc) throws IOException {
return true;
}
+ /** Mirrors {@code HistogramCollector#advanceSkipper}. */
+ private void advanceSkipper(int doc) throws IOException {
+ if (doc > skipper.maxDocID(0)) {
+ skipper.advance(doc);
+ }
+ upToSameInterval = false;
+
+ if (skipper.minDocID(0) > doc) {
+ // Corner case which happens if doc doesn't have a value and is
between two intervals of the
+ // skip index. Fall back to per-doc lookups until the next block.
+ upToInclusive = skipper.minDocID(0) - 1;
+ return;
+ }
+
+ upToInclusive = skipper.maxDocID(0);
+ // Now find the highest level where all docs have a value and map to the
same interval.
+ for (int level = 0; level < skipper.numLevels(); ++level) {
+ int totalDocsAtLevel = skipper.maxDocID(level) -
skipper.minDocID(level) + 1;
+ if (skipper.docCount(level) != totalDocsAtLevel) {
+ // Some docs at this level have no value, so we can't resolve the
whole block at once.
Review Comment:
Ran: ` python3 -u src/python/localrun.py -s rangeFacetsWikimediumAll -b
lucene_baseline -c lucene_candidate -iterations 30 -warmups 20 2>&1 | tee
"$BASE/run9-onlyunwrapping-timing.txt"`
Heres the result.
## Latency (ms) — aggregated across all iterations
| Task | P50 B | P50 C | Diff | P90 B | P90 C | Diff | P99 B | P99 C |
Diff | P999 B | P999 C | Diff | P100 B | P100 C | Diff |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| BrowseIDOvlpRangeFacets | 946.928 | 820.170 | -13.4% | 1279.722 |
1178.839 | -7.9% | 1509.599 | 3489.425 | +131.1% | 1579.256 | 10588.392 |
+570.5% | 1647.232 | 10662.196 | +547.3% |
| BrowseIDRangeFacets | 410.635 | 296.512 | -27.8% | 630.174 | 670.131 |
+6.3% | 869.641 | 866.369 | -0.4% | 931.130 | 8681.360 | +832.3% | 1008.106 |
9240.540 | +816.6% |
| BrowseLastModOvlpRangeFacets | 381.116 | 365.262 | -4.2% | 577.681 |
639.843 | +10.8% | 866.191 | 2046.696 | +136.3% | 1028.218 | 10214.331 |
+893.4% | 1029.600 | 10234.017 | +894.0% |
| BrowseLastModRangeFacets | 324.172 | 317.631 | -2.0% | 526.130 | 634.818
| +20.7% | 808.836 | 884.852 | +9.4% | 851.734 | 9413.995 | +1005.3% | 887.375
| 9912.493 | +1017.1% |
| MedTermIDOvlpRangeFacets | 213.936 | 181.524 | -15.1% | 433.905 |
437.942 | +0.9% | 603.100 | 702.917 | +16.6% | 679.574 | 838.447 | +23.4% |
761.983 | 848.642 | +11.4% |
| MedTermIDRangeFacets | 211.334 | 171.214 | -19.0% | 519.706 | 555.351 |
+6.9% | 735.139 | 713.797 | -2.9% | 834.724 | 836.369 | +0.2% | 840.486 |
843.231 | +0.3% |
| MedTermLastModOvlpRangeFacets | 176.353 | 173.086 | -1.9% | 486.851 |
537.847 | +10.5% | 712.630 | 719.706 | +1.0% | 830.068 | 832.742 | +0.3% |
842.260 | 840.165 | -0.2% |
| MedTermLastModRangeFacets | 188.926 | 182.942 | -3.2% | 487.783 |
553.020 | +13.4% | 731.373 | 718.764 | -1.7% | 830.363 | 832.185 | +0.2% |
841.503 | 834.076 | -0.9% |
## QPS
| Task | QPS baseline | StdDev | QPS modified | StdDev | Pct diff |
p-value |
|---|---|---|---|---|---|---|
| BrowseLastModRangeFacets | 3.26 | (5.8%) | 3.34 | (8.1%) | 2.6% (-10% -
17%) | 0.155 |
| BrowseLastModOvlpRangeFacets | 2.75 | (5.1%) | 2.85 | (6.2%) | 3.6% (-7%
- 15%) | 0.015 |
| MedTermLastModRangeFacets | 5.58 | (7.9%) | 5.80 | (8.6%) | 3.8% (-11% -
22%) | 0.072 |
| MedTermLastModOvlpRangeFacets | 5.89 | (6.9%) | 6.13 | (8.2%) | 4.1%
(-10% - 20%) | 0.035 |
| MedTermIDOvlpRangeFacets | 4.94 | (6.3%) | 5.68 | (6.6%) | 15.0% (1% -
29%) | 0.000 |
| MedTermIDRangeFacets | 5.19 | (9.9%) | 6.17 | (7.1%) | 18.7% (1% - 39%)
| 0.000 |
| BrowseIDOvlpRangeFacets | 1.12 | (7.4%) | 1.33 | (11.9%) | 19.1% (0% -
41%) | 0.000 |
| BrowseIDRangeFacets | 2.52 | (4.5%) | 3.60 | (12.0%) | 42.7% (25% - 61%)
| 0.000 |
Mainly impacts the ID tasks which do not have a skipper.
But it does seem to cause a worrying latency regression at high percentile
latency (p90 and onward).
Hmm, @epotyom what do you think? Since it is not related to the skipper
change, I am partial towards removing the unwrapping and retesting performance.
--
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]
