sam-1112 commented on PR #5960: URL: https://github.com/apache/datafusion-comet/pull/5960#issuecomment-5685778126
Thanks Andy. I expanded the existing Criterion benchmark to cover the compatible type families and the fallback paths involved in this change. Compared revisions: - Base: `fad623094` (`main`) - PR: `a1152566e` ## Benchmark coverage The benchmark contains 17 cases covering batch size, null distribution, compatible type families, and fallback paths. The multi-column cases hash an `Int64 + Utf8 + Float64` key using seed 42 across: - 8,192, 65,536, and 524,288 rows - No nulls, 10% sparse nulls, and all-null inputs Additional 8,192-row cases cover: - Primitive `Int64` - `Utf8 + Binary` - `Decimal128(10, 2) + Decimal128(38, 10)` - `Dictionary<Int32, Utf8>` - `List<Int32>` with 10 elements per row - `Map<Utf8, Int32>` with 10 entries per row - `Struct<Int32, Utf8>` - `Int64` with custom seed 7 The seed-42 compatible cases exercise the new `SparkXxhash64` route. The struct and custom-seed cases exercise the retained Comet fallback path. ## Multi-column batch-size and null-distribution results Lower execution time is better. Negative changes indicate improvement. | Rows | Null distribution | `main` time | PR time | Change (95% CI) | Result | |---:|:---|:---|:---|:---|:---| | 8,192 | No nulls | 34.059–35.348 µs | 30.711–30.830 µs | −11.33% to −8.62% (estimate: −9.92%) | Improved | | 8,192 | Sparse nulls (10%) | 36.277–36.843 µs | 34.888–35.794 µs | −14.69% to −5.37% (estimate: −9.32%) | Improved | | 8,192 | All null | 13.730–13.768 µs | 11.554–11.753 µs | −16.12% to −15.16% (estimate: −15.76%) | Improved | | 65,536 | No nulls | 247.41–253.53 µs | 230.53–234.15 µs | −7.88% to −6.00% (estimate: −6.99%) | Improved | | 65,536 | Sparse nulls (10%) | 279.18–283.42 µs | 273.89–277.78 µs | −2.99% to +0.16% (estimate: −1.43%) | No significant change | | 65,536 | All null | 110.86–111.10 µs | 90.088–90.685 µs | −18.98% to −18.61% (estimate: −18.80%) | Improved | | 524,288 | No nulls | 2.0139–2.0662 ms | 1.8504–1.8736 ms | −10.12% to −7.61% (estimate: −8.71%) | Improved | | 524,288 | Sparse nulls (10%) | 2.2451–2.2981 ms | 2.2126–2.2875 ms | −2.90% to +1.32% (estimate: −0.89%) | No significant change | | 524,288 | All null | 898.35–911.73 µs | 731.02–735.89 µs | −21.99% to −16.92% (estimate: −19.27%) | Improved | ## Type-family and fallback-path results This table shows the latest paired run using a freshly captured baseline and the same filtered case order on both revisions. | Route | Shape | `main` time | PR time | Change (95% CI) | Result | |:---|:---|:---|:---|:---|:---| | Compatible | Primitive `Int64` | 10.780–10.880 µs | 8.492–8.645 µs | −23.13% to −21.67% (estimate: −22.35%) | Improved | | Compatible | `Utf8 + Binary` | 35.306–35.361 µs | 37.734–38.508 µs | +6.69% to +8.42% (estimate: +7.45%) | Regressed in this run | | Compatible | Narrow + wide `Decimal128` | 23.178–23.629 µs | 21.054–22.852 µs | −12.23% to −7.70% (estimate: −10.14%) | Improved | | Compatible | `Dictionary<Int32, Utf8>` | 12.921–13.131 µs | 8.483–8.909 µs | −36.30% to −33.06% (estimate: −34.65%) | Improved | | Compatible | `List<Int32>` × 10 | 154.07–154.77 µs | 144.16–145.99 µs | −5.78% to −3.78% (estimate: −4.80%) | Improved | | Compatible | `Map<Utf8, Int32>` × 10 | 626.28–641.70 µs | 617.40–633.18 µs | −9.39% to −3.55% (estimate: −6.15%) | Improved | | Fallback | `Struct<Int32, Utf8>` | 26.921–27.027 µs | 27.131–27.759 µs | −3.99% to +2.26% (estimate: −0.35%) | No significant change | | Fallback | `Int64`, custom seed 7 | 9.608–9.642 µs | 9.660–9.733 µs | +0.17% to +0.84% (estimate: +0.52%) | Within Criterion's noise threshold | ## Repeatability I repeated the type-family comparison because the local macOS runs showed cross-process variation. The following changes were reproducible: - Primitive `Int64`: approximately 15–22% faster - Narrow + wide Decimal128: approximately 9–10% faster - Dictionary: approximately 34–35% faster - All-null multi-column cases: approximately 14–19% faster Some cases changed direction between independent runs: - `Utf8 + Binary`: approximately −2% to +7.5% - `List<Int32>`: unchanged to approximately 4.8% faster - `Map<Utf8, Int32>`: approximately 6.1% faster to 4.1% slower - Struct fallback: approximately 6.4% faster to 2.7% slower - Custom-seed fallback: unchanged in two comparisons, with one noisy run reporting approximately 8.7% slower Because these are separate Criterion processes on a local macOS machine, I would treat the inconsistent cases as inconclusive rather than claim a reproducible improvement or regression. A dedicated Linux benchmark host would be appropriate if we need a firmer conclusion for those cases. Overall, the clearest reproducible wins are primitive, Decimal128, and dictionary inputs. The fallback paths do not show a consistent material change. -- 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]
