serhiy-bzhezytskyy commented on PR #16411:
URL: https://github.com/apache/lucene/pull/16411#issuecomment-5096350679
Ran the benchmark. Short answer: no measurable difference on this hardware —
not one task in the suite came in below p=0.05, including the sort tasks
themselves.
Setup: luceneutil `wikimedium10m`, baseline `9d09aa71eef` (this branch's
parent, rather than current `main`, so the one unrelated commit ahead doesn't
fold into the delta) vs candidate `af9873067eb`. Both competitors search the
same index, 20 JVM iterations, 20 warmups.
```
TaskQPS baseline StdDev QPS candidate StdDev
Pct diff p-value
HighTermTitleSort 154.09 (10.2%) 151.53 (12.0%)
-1.7% 0.638
HighTermTitleBDVSort 43.32 (9.0%) 44.56 (11.7%)
+2.9% 0.385
HighTermDayOfYearSort 448.00 (16.8%) 448.09 (18.8%)
0.0% 0.997
TermDTSort 440.18 (15.7%) 447.19 (16.2%)
+1.6% 0.752
PKLookup 551.21 (2.2%) 545.92 (3.9%)
-1.0% 0.339
Respell 97.71 (2.9%) 97.17 (4.5%)
-0.5% 0.647
```
Whole-suite spread was `BrowseDateSSDVFacets -8.5%` to `AndMissingHigh
+8.1%`, with median per-task stddev 13.3%. The `-1.7%` on `HighTermTitleSort`
sits well inside its own 10-12% deviation, so I'd read it as no effect rather
than as a small cost — and `HighTermTitleBDVSort`, which sorts the same field
without going through the changed comparator, moved the same amount in the
other direction.
One caveat on how much this is worth: this is a laptop, and a 13.3% median
floor can't rule out a small single-digit cost. An earlier run of the same two
shas on the same index gave `HighTermTitleSort -45.1%` at p=0.002 — that turned
out to be a stray process eating 5+ cores, and after quiescing the machine it
became the -1.7% above. So I'd treat this as "nothing showed up at 20
iterations on one machine" rather than a tight bound. If a run on the nightly
hardware would be more convincing, that seems worth doing before trusting the
number.
@romseygeek on extending to the single-value and missing-first cases —
#16424 looks like it covers that ground properly, and I'd rather not duplicate
it. Its only overlap with this PR is `CHANGES.txt`. If it'd be useful once that
lands, I can check the missing-field case against it, since that's the scenario
this patch targets and it isn't in the standard task set.
--
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]
