sunchao commented on code in PR #5380: URL: https://github.com/apache/datafusion-comet/pull/5380#discussion_r3792003267
########## benchmarks/results/micro/CometAggregateExpressionBenchmark-results.txt: ########## @@ -0,0 +1,390 @@ +================================================================================================ Review Comment: **[P1] Handle generated results in both Apache RAT validation paths.** All 35 new `*-results.txt` files lack recognized ASF headers, and the existing Maven exclusion `**/benchmarks/*.txt` does not match `benchmarks/results/micro/`. The current exact-head Preflight job already fails with `Unapproved: 35` / `Too many files with unapproved license: 35`, which skips the downstream build, benchmark, and Spark/Iceberg test jobs. Source releases also run a separate RAT filter using `dev/release/rat_exclude_files.txt`, so changing only `pom.xml` would still leave release creation broken. Please add narrowly scoped generated-result exclusions to **both** validation paths, or emit recognized ASF headers for every result file. ########## benchmarks/results/micro/RUN-INFO.md: ########## @@ -0,0 +1,80 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Comet Micro Benchmark Run + +Generated by `benchmarks/micro/run.py collect`. + +## Environment + +| Field | Value | Review Comment: **[P2] Add the missing formatter exemption for the generated run manifest.** `RUN-INFO.md` contains unaligned generated Markdown tables, but this PR does not add it to `.prettierignore`; the exact-head Preflight workflow runs `prettier --check "**/*.md"` immediately after the RAT step. The prerequisite runner branch already contains a dedicated commit adding `benchmarks/results/micro/RUN-INFO.md` to `.prettierignore` because this manifest is generated on a machine without Prettier, but that exemption was omitted here. Consequently fixing the current RAT failure would expose a second Markdown-formatting failure. Please include the prerequisite ignore entry or format and preserve the generated manifest accordingly. ########## benchmarks/results/micro/README.md: ########## @@ -0,0 +1,34 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Micro Benchmark Results + +This directory holds published results from the Comet micro benchmark suites in +`spark/src/test/scala/org/apache/spark/sql/benchmark`. + +`RUN-INFO.md` records the machine, the Comet commit, and the per-suite status of the run that +produced the results files next to it. Results are only comparable across runs on the same +instance type, so replace the whole directory in one commit rather than updating individual files. + +Results are produced and collected with `benchmarks/micro/run.py`. See Review Comment: **[P2] Merge the benchmark runner and setup before publishing its outputs.** Neither the documented `benchmarks/micro/run.py` nor the linked `docs/source/contributor-guide/benchmarking_micro_ec2.md` exists at this PR head; both paths return 404. The measured commit recorded in `RUN-INFO.md` belongs to the separate, still-open runner PR #5374, which also contains the configurable `BENCH_HEAP` support needed to reproduce the documented 8g heap; this PR's Makefile still hardcodes `-Xmx20g`. As submitted, contributors cannot run the documented collection workflow or reproduce the recorded configuration from this tree. Please merge/include the prerequisite tooling first, or provide working in-tree instructions and equivalent configuration. ########## benchmarks/results/micro/CometCastTemporalToNumericBenchmark-results.txt: ########## @@ -0,0 +1,32 @@ +================================================================================================ +Date to Numeric casts +================================================================================================ + +OpenJDK 64-Bit Server VM 17.0.20+8-LTS on Linux 6.18.39-79.141.amzn2023.x86_64 +Intel(R) Xeon(R) Platinum 8488C +CAST Date to BYTE: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative Review Comment: **[P2] Remove always-null DATE casts from the conversion baseline.** The benchmark disables ANSI mode, under which Spark casts from `DATE` to `BYTE`, `SHORT`, `INT`, and `LONG` always return `NULL`; Comet explicitly special-cases the same always-null conversions. Consequently all four reported 1.2x-1.3x results in this file measure scanning and null materialization rather than any date-to-number conversion, so they cannot detect regressions in real temporal conversion. Please replace them with a meaningful operation such as `unix_date(c_date)`, or omit these misleading cases from the published baseline. ########## benchmarks/results/micro/RUN-INFO.md: ########## @@ -0,0 +1,80 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Comet Micro Benchmark Run + +Generated by `benchmarks/micro/run.py collect`. + +## Environment + +| Field | Value | +| --- | --- | +| Date (UTC) | 2026-08-15T23:03:22.854256+00:00 | +| Instance type | m7i.2xlarge | +| Availability zone | us-east-1d | +| CPU | Intel(R) Xeon(R) Platinum 8488C (8 vCPU) | +| Memory | 30.8 GiB | +| OS | Linux-6.18.39-79.141.amzn2023.x86_64-x86_64-with-glibc2.34 | +| Java | openjdk version "17.0.20" 2026-07-21 LTS | +| Rust | rustc 1.97.1 (8bab26f4f 2026-07-14) | +| Maven profile | default | +| Benchmark heap | 8g | +| Comet branch | micro-benchmark-ec2 | +| Comet commit | 3392ebb9a34c769380190693b197aa5e295c6424 | +| Total run time | 2h29m23s | + +## Suites + +| Suite | Status | Duration | Results file | +| --- | --- | --- | --- | +| CometAggregateExpressionBenchmark | ok | 8m07s | CometAggregateExpressionBenchmark-results.txt | +| CometArithmeticBenchmark | ok | 5m24s | CometArithmeticBenchmark-results.txt | +| CometArrayExpressionBenchmark | ok | 2m42s | CometArrayExpressionBenchmark-results.txt | +| CometBroadcastHashJoinBenchmark | ok | 51s | CometBroadcastHashJoinBenchmark-results.txt | +| CometBroadcastNestedLoopJoinBenchmark | ok | 43s | CometBroadcastNestedLoopJoinBenchmark-results.txt | +| CometCastBooleanBenchmark | ok | 4m17s | CometCastBooleanBenchmark-results.txt | +| CometCastNumericToNumericBenchmark | ok | 12m24s | CometCastNumericToNumericBenchmark-results.txt | +| CometCastNumericToStringBenchmark | ok | 2m26s | CometCastNumericToStringBenchmark-results.txt | +| CometCastNumericToTemporalBenchmark | failed | 7s | CometCastNumericToTemporalBenchmark-results.txt | +| CometCastStringToNumericBenchmark | ok | 3m15s | CometCastStringToNumericBenchmark-results.txt | +| CometCastStringToTemporalBenchmark | ok | 45s | CometCastStringToTemporalBenchmark-results.txt | +| CometCastTemporalToNumericBenchmark | failed | 41s | CometCastTemporalToNumericBenchmark-results.txt | +| CometCastTemporalToStringBenchmark | ok | 44s | CometCastTemporalToStringBenchmark-results.txt | +| CometCastTemporalToTemporalBenchmark | ok | 42s | CometCastTemporalToTemporalBenchmark-results.txt | +| CometColumnarToRowBenchmark | ok | 2m35s | CometColumnarToRowBenchmark-results.txt | +| CometComparisonExpressionBenchmark | ok | 2m21s | CometComparisonExpressionBenchmark-results.txt | +| CometConditionalExpressionBenchmark | ok | 1m19s | CometConditionalExpressionBenchmark-results.txt | +| CometCsvExpressionBenchmark | ok | 17s | CometCsvExpressionBenchmark-results.txt | +| CometDatetimeExpressionBenchmark | ok | 2m55s | CometDatetimeExpressionBenchmark-results.txt | +| CometExecBenchmark | ok | 2m49s | CometExecBenchmark-results.txt | +| CometGetJsonObjectBenchmark | ok | 1m03s | CometGetJsonObjectBenchmark-results.txt | +| CometHashExpressionBenchmark | ok | 4m29s | CometHashExpressionBenchmark-results.txt | +| CometHashJoinBenchmark | ok | 51s | CometHashJoinBenchmark-results.txt | +| CometIcebergReadBenchmark | ok | 2m23s | CometIcebergReadBenchmark-results.txt | +| CometJsonExpressionBenchmark | ok | 2m32s | CometJsonExpressionBenchmark-results.txt | +| CometLengthOfJsonArrayBenchmark | ok | 24s | CometLengthOfJsonArrayBenchmark-results.txt | +| CometOperatorSerdeBenchmark | ok | 6m45s | CometOperatorSerdeBenchmark-results.txt | +| CometPartitionColumnBenchmark | ok | 47s | CometPartitionColumnBenchmark-results.txt | +| CometPredicateExpressionBenchmark | ok | 16s | CometPredicateExpressionBenchmark-results.txt | +| CometReadBenchmark | failed | 3m54s | CometReadBenchmark-results.txt | +| CometRegExpBenchmark | ok | 1m29s | CometRegExpBenchmark-results.txt | +| CometRegExpExtractBenchmark | ok | 3m37s | CometRegExpExtractBenchmark-results.txt | +| CometShuffleBenchmark | timeout | 1h00m00s | CometShuffleBenchmark-results.txt | Review Comment: **[P2] Complete or shard the shuffle benchmark before publishing its baseline.** The shuffle suite hit its one-hour timeout while still processing struct cases. Its committed results therefore omit all subsequently scheduled dictionary shuffle, ordinary and randomized shuffle, wide shuffle, and wide range-partition shuffle groups, including the native-shuffle comparisons those groups contain. This is a substantial coverage gap in precisely the workload family this baseline should protect, not merely a slower final sample. Please increase the suite timeout or split/shard the workload, then collect a completed shuffle artifact. ########## benchmarks/results/micro/CometOperatorSerdeBenchmark-results.txt: ########## @@ -0,0 +1,25 @@ +================================================================================================ +CometScanRule Benchmark +================================================================================================ + +OpenJDK 64-Bit Server VM 17.0.20+8-LTS on Linux 6.18.39-79.141.amzn2023.x86_64 +Intel(R) Xeon(R) Platinum 8488C +CometScanRule apply (30000 partitions): Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative +------------------------------------------------------------------------------------------------------------------------ +CometScanRule.apply(sparkPlan) 1577 1585 12 0.0 15766703.4 1.0X + + +================================================================================================ +IcebergScan Operator Serde Benchmark +================================================================================================ + +OpenJDK 64-Bit Server VM 17.0.20+8-LTS on Linux 6.18.39-79.141.amzn2023.x86_64 +Intel(R) Xeon(R) Platinum 8488C +IcebergScan serde (30000 partitions, 30000 tasks): Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative Review Comment: **[P2] Do not label placeholder serialization as 30,000-task Iceberg serde.** The benchmark obtains 30,000 `FileScanTask`s but measures `CometIcebergNativeScan.convert(...)` and serialization of its returned operator. Both the exact PR-head implementation and the measured source commit explicitly document that `convert` creates only a minimal metadata placeholder with an empty partition field; actual task and partition serialization occurs later in `serializePartitions()`, which these cases never invoke. The reported 102-691 ns/op figures therefore serialize **zero** of the claimed 30,000 tasks and materially misrepresent the real task-scaling cost. Please benchmark `serializePartitions()`/the actual task payload, or relabel these rows as constant-size placeholder conversion. ########## benchmarks/results/micro/RUN-INFO.md: ########## @@ -0,0 +1,80 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Comet Micro Benchmark Run + +Generated by `benchmarks/micro/run.py collect`. + +## Environment + +| Field | Value | +| --- | --- | +| Date (UTC) | 2026-08-15T23:03:22.854256+00:00 | +| Instance type | m7i.2xlarge | +| Availability zone | us-east-1d | +| CPU | Intel(R) Xeon(R) Platinum 8488C (8 vCPU) | +| Memory | 30.8 GiB | +| OS | Linux-6.18.39-79.141.amzn2023.x86_64-x86_64-with-glibc2.34 | +| Java | openjdk version "17.0.20" 2026-07-21 LTS | +| Rust | rustc 1.97.1 (8bab26f4f 2026-07-14) | +| Maven profile | default | +| Benchmark heap | 8g | +| Comet branch | micro-benchmark-ec2 | +| Comet commit | 3392ebb9a34c769380190693b197aa5e295c6424 | +| Total run time | 2h29m23s | + +## Suites + +| Suite | Status | Duration | Results file | +| --- | --- | --- | --- | +| CometAggregateExpressionBenchmark | ok | 8m07s | CometAggregateExpressionBenchmark-results.txt | +| CometArithmeticBenchmark | ok | 5m24s | CometArithmeticBenchmark-results.txt | +| CometArrayExpressionBenchmark | ok | 2m42s | CometArrayExpressionBenchmark-results.txt | +| CometBroadcastHashJoinBenchmark | ok | 51s | CometBroadcastHashJoinBenchmark-results.txt | +| CometBroadcastNestedLoopJoinBenchmark | ok | 43s | CometBroadcastNestedLoopJoinBenchmark-results.txt | +| CometCastBooleanBenchmark | ok | 4m17s | CometCastBooleanBenchmark-results.txt | +| CometCastNumericToNumericBenchmark | ok | 12m24s | CometCastNumericToNumericBenchmark-results.txt | +| CometCastNumericToStringBenchmark | ok | 2m26s | CometCastNumericToStringBenchmark-results.txt | +| CometCastNumericToTemporalBenchmark | failed | 7s | CometCastNumericToTemporalBenchmark-results.txt | Review Comment: **[P2] Do not publish failed suites as completed regression baselines.** Three listed suites failed but their partial output was nevertheless committed: `CometCastNumericToTemporalBenchmark-results.txt` contains only a four-line heading and zero measurements; `CometCastTemporalToNumericBenchmark-results.txt` contains only 4 of its 16 defined cases; and `CometReadBenchmark-results.txt` stops at the encrypted-scan heading, omitting the remaining encrypted, decimal, dictionary, filter, wide-column, and string-scan groups. Keeping these files beside successful artifacts makes entire benchmark families unavailable for future comparisons while presenting them as published baselines. Please fix and rerun the failed suites, or exclude their artifacts until valid results can be collected. -- 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]
