You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2024/07/15 22:52:24 UTC
[PR] chore: Add microbenchmarks [datafusion-comet]
andygrove opened a new pull request, #671:
URL: https://github.com/apache/datafusion-comet/pull/671
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
N/A
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
When running the complex TPC-* queries, it is challenging to debug why Comet is slower in some cases.
I think it would be easier if we had some simpler queries to test with that only test a smaller number of operators and expressions. We can base these queries on subsets of the real TPC queries.
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
New queries and a PySpark app for running them
## How are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2230835502
@parthchandra @kazuyukitanimura @huaxingao @viirya This is ready for review now
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#discussion_r1681640222
##########
spark/src/test/resources/tpcds-micro-benchmarks/add_many_decimals.sql:
##########
@@ -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.
+
+-- This is testing the cost of a complex expression that will create many intermediate arrays in Comet
Review Comment:
the query includes an expression adding many columns e.g. `a+b+c+d...` and we will compute this by recursively computing binary expressions and creating an intermediate array for each one, something like:
```
temp1 = a+b
temp2 = temp1+c
temp3 = temp2+d
...
```
The Spark version will generate and compile code that just performs `a+b+c+d..` without creating intermediate output.
We could potentially explore using jit or introduce specialized expressions that re-use intermediate buffers in some cases.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2233705069
> I tried integrating into the existing suite but the queries are not running correctly and I am not sure why.
>
> Example:
>
> ```
> TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
> ------------------------------------------------------------------------------------------------------------------------
> add_many_decimals 15 21 7 0.0 Infinity 1.0X
> add_many_decimals 13 16 3 0.0 Infinity 1.2X
> add_many_decimals: Comet (Scan) 13 15 3 0.0 Infinity 1.2X
> add_many_decimals: Comet (Scan, Exec) 11 12 2 0.0 Infinity 1.4X
> ```
>
> This query should take more than 20 seconds to run
You were using the same scale factor I presume?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2229625195
## [Codecov](https://app.codecov.io/gh/apache/datafusion-comet/pull/671?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 53.88%. Comparing base [(`b6d868c`)](https://app.codecov.io/gh/apache/datafusion-comet/commit/b6d868c050ce21fd16641063d7be917e63d66e6b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`e2e2bd3`)](https://app.codecov.io/gh/apache/datafusion-comet/commit/e2e2bd34b68001ca69db2a76f8c2385515d91eeb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 6 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #671 +/- ##
=============================================
+ Coverage 33.65% 53.88% +20.23%
+ Complexity 842 811 -31
=============================================
Files 109 106 -3
Lines 42573 10238 -32335
Branches 9334 1916 -7418
=============================================
- Hits 14328 5517 -8811
+ Misses 25297 3744 -21553
+ Partials 2948 977 -1971
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/datafusion-comet/pull/671?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2234226194
> However, this path would be hit only for precision > 18 or if `spark.comet.use.decimal128` was set to `true` (it is `false` by default).
The fields have precision 7 and I am not aware of `spark.comet.use.decimal128` being set.
```
| ss_wholesale_cost | Decimal128(7, 2) | YES |
| ss_list_price | Decimal128(7, 2) | YES |
| ss_sales_price | Decimal128(7, 2) | YES |
| ss_ext_discount_amt | Decimal128(7, 2) | YES |
| ss_ext_sales_price | Decimal128(7, 2) | YES |
| ss_ext_wholesale_cost | Decimal128(7, 2) | YES |
| ss_ext_list_price | Decimal128(7, 2) | YES |
| ss_ext_tax | Decimal128(7, 2) | YES |
| ss_coupon_amt | Decimal128(7, 2) | YES |
| ss_net_paid | Decimal128(7, 2) | YES |
| ss_net_paid_inc_tax | Decimal128(7, 2) | YES |
| ss_net_profit | Decimal128(7, 2) | YES |
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2234106564
For reference, here are the results from running with sf=100gb data on my Linux workstation.
```
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
add_many_decimals 20502 20648 208 14.0 71.2 1.0X
add_many_decimals 20498 20544 65 14.1 71.2 1.0X
add_many_decimals: Comet (Scan) 28143 28161 26 10.2 97.7 0.7X
add_many_decimals: Comet (Scan, Exec) 19323 19497 246 14.9 67.1 1.1X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
add_many_integers 2083 2116 45 138.2 7.2 1.0X
add_many_integers 2146 2177 43 134.2 7.5 1.0X
add_many_integers: Comet (Scan) 1879 1929 71 153.3 6.5 1.1X
add_many_integers: Comet (Scan, Exec) 2102 2108 8 137.0 7.3 1.0X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
agg_high_cardinality 1280 1306 37 56.3 17.8 1.0X
agg_high_cardinality 1135 1141 8 63.4 15.8 1.1X
agg_high_cardinality: Comet (Scan) 1612 1656 64 44.7 22.4 0.8X
agg_high_cardinality: Comet (Scan, Exec) 770 822 74 93.5 10.7 1.7X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
agg_low_cardinality 255 276 16 282.2 3.5 1.0X
agg_low_cardinality 255 274 18 282.7 3.5 1.0X
agg_low_cardinality: Comet (Scan) 738 745 7 97.5 10.3 0.3X
agg_low_cardinality: Comet (Scan, Exec) 191 213 16 377.4 2.6 1.3X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------------
agg_sum_decimals_no_grouping 10552 10583 44 27.3 36.6 1.0X
agg_sum_decimals_no_grouping 10406 10450 61 27.7 36.1 1.0X
agg_sum_decimals_no_grouping: Comet (Scan) 46013 46278 375 6.3 159.8 0.2X
agg_sum_decimals_no_grouping: Comet (Scan, Exec) 13840 13956 164 20.8 48.1 0.8X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------------
agg_sum_integers_no_grouping 2220 2251 45 129.8 7.7 1.0X
agg_sum_integers_no_grouping 2167 2245 110 132.9 7.5 1.0X
agg_sum_integers_no_grouping: Comet (Scan) 1984 2009 37 145.2 6.9 1.1X
agg_sum_integers_no_grouping: Comet (Scan, Exec) 2090 2108 26 137.8 7.3 1.1X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
----------------------------------------------------------------------------------------------------------------------------
case_when_column_or_null 963 981 21 298.9 3.3 1.0X
case_when_column_or_null 941 944 6 306.1 3.3 1.0X
case_when_column_or_null: Comet (Scan) 2697 2699 4 106.8 9.4 0.4X
case_when_column_or_null: Comet (Scan, Exec) 1235 1242 9 233.2 4.3 0.8X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
case_when_scalar 201 213 7 359.1 2.8 1.0X
case_when_scalar 200 214 10 360.7 2.8 1.0X
case_when_scalar: Comet (Scan) 1856 1889 46 38.8 25.8 0.1X
case_when_scalar: Comet (Scan, Exec) 390 409 12 184.6 5.4 0.5X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
---------------------------------------------------------------------------------------------------------------------------
filter_highly_selective 197 212 10 366.1 2.7 1.0X
filter_highly_selective 193 205 5 372.5 2.7 1.0X
filter_highly_selective: Comet (Scan) 1146 1148 3 62.8 15.9 0.2X
filter_highly_selective: Comet (Scan, Exec) 112 127 11 645.4 1.5 1.8X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
-------------------------------------------------------------------------------------------------------------------------
filter_less_selective 198 207 6 363.4 2.8 1.0X
filter_less_selective 196 211 11 367.2 2.7 1.0X
filter_less_selective: Comet (Scan) 871 879 10 82.7 12.1 0.2X
filter_less_selective: Comet (Scan, Exec) 127 144 11 565.6 1.8 1.6X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
if_column_or_null 615 636 32 468.5 2.1 1.0X
if_column_or_null 599 620 26 481.3 2.1 1.0X
if_column_or_null: Comet (Scan) 907 921 18 317.8 3.1 0.7X
if_column_or_null: Comet (Scan, Exec) 1022 1038 23 281.8 3.5 0.6X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
join_anti 4460 4530 98 16.1 62.0 1.0X
join_anti 3766 3907 200 19.1 52.3 1.2X
join_anti: Comet (Scan) 4025 4112 123 17.9 55.9 1.1X
join_anti: Comet (Scan, Exec) 4035 4085 70 17.8 56.0 1.1X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
join_condition 1604 1650 65 338.8 3.0 1.0X
join_condition 1614 1621 10 336.7 3.0 1.0X
join_condition: Comet (Scan) 1466 1484 26 370.6 2.7 1.1X
join_condition: Comet (Scan, Exec) 1688 1694 10 321.9 3.1 1.0X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
-------------------------------------------------------------------------------------------------------------------------
join_exploding_output 1476 1512 51 368.1 2.7 1.0X
join_exploding_output 1461 1487 37 371.9 2.7 1.0X
join_exploding_output: Comet (Scan) 1346 1381 50 403.7 2.5 1.1X
join_exploding_output: Comet (Scan, Exec) 1459 1468 13 372.5 2.7 1.0X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
join_inner 213 221 5 1350.9 0.7 1.0X
join_inner 209 217 6 1378.2 0.7 1.0X
join_inner: Comet (Scan) 204 208 3 1411.9 0.7 1.0X
join_inner: Comet (Scan, Exec) 314 318 4 917.6 1.1 0.7X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
join_left_outer 88655 90623 2784 3.6 279.9 1.0X
join_left_outer 87672 88273 850 3.6 276.7 1.0X
join_left_outer: Comet (Scan) 87751 89203 2054 3.6 277.0 1.0X
join_left_outer: Comet (Scan, Exec) 89906 92005 2969 3.5 283.8 1.0X
OpenJDK 64-Bit Server VM 11.0.23+9-post-Ubuntu-1ubuntu122.04.1 on Linux 6.5.0-41-generic
AMD Ryzen 9 7950X3D 16-Core Processor
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
join_semi 9258 9871 867 7.8 128.6 1.0X
join_semi 9176 9503 462 7.8 127.4 1.0X
join_semi: Comet (Scan) 9519 9701 257 7.6 132.2 1.0X
join_semi: Comet (Scan, Exec) 8726 9192 659 8.3 121.2 1.1X
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#discussion_r1681869984
##########
spark/src/test/scala/org/apache/spark/sql/benchmark/CometTPCDSMicroBenchmark.scala:
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.benchmark
+
+import scala.io.Source
+
+import org.apache.spark.benchmark.Benchmark
+import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
+import org.apache.spark.sql.catalyst.plans.logical.SubqueryAlias
+import org.apache.spark.sql.execution.benchmark.TPCDSQueryBenchmark.tables
+import org.apache.spark.sql.execution.benchmark.TPCDSQueryBenchmarkArguments
+import org.apache.spark.sql.execution.datasources.LogicalRelation
+
+import org.apache.comet.CometConf
+
+/**
+ * Benchmark to measure Comet query performance with micro benchmarks that use the TPCDS data.
+ * These queries represent subsets of the full TPCDS queries.
+ *
+ * To run this benchmark:
+ * {{{
+ * // Build [tpcds-kit](https://github.com/databricks/tpcds-kit)
+ * cd /tmp && git clone https://github.com/databricks/tpcds-kit.git
+ * cd tpcds-kit/tools && make OS=MACOS
+ *
+ * // GenTPCDSData
+ * cd $COMET_HOME && mkdir /tmp/tpcds
+ * make benchmark-org.apache.spark.sql.GenTPCDSData -- --dsdgenDir /tmp/tpcds-kit/tools --location /tmp/tpcds --scaleFactor 1
+ *
+ * // CometTPCDSMicroBenchmark
+ * SPARK_GENERATE_BENCHMARK_FILES=1 make benchmark-org.apache.spark.sql.benchmark.CometTPCDSMicroBenchmark -- --data-location /tmp/tpcds
+ * }}}
+ *
+ * Results will be written to "spark/benchmarks/CometTPCDSMicroBenchmark-**results.txt".
+ */
+object CometTPCDSMicroBenchmark extends CometTPCQueryBenchmarkBase {
+
+ val queries: Seq[String] = Seq(
+ "add_many_decimals",
+ "add_many_integers",
+ "agg_high_cardinality",
+ "agg_low_cardinality",
+ "agg_sum_decimals_no_grouping",
+ "agg_sum_integers_no_grouping",
+ "case_when_column_or_null",
+ "case_when_scalar",
+ "filter_highly_selective",
+ "filter_less_selective",
+ "if_column_or_null",
+ "join_anti",
+ "join_condition",
+ "join_exploding_output",
+ "join_inner",
+ "join_left_outer",
+ "join_semi")
+
+ override def runQueries(
+ queryLocation: String,
+ queries: Seq[String],
+ tableSizes: Map[String, Long],
+ benchmarkName: String,
+ nameSuffix: String = ""): Unit = {
+ queries.foreach { name =>
+ val source = Source.fromFile(s"spark/src/test/resources/tpcds-micro-benchmarks/$name.sql")
+ val queryString = source
+ .getLines()
+ .filterNot(_.startsWith("--"))
+ .mkString("\n")
+ source.close()
+
+ // This is an indirect hack to estimate the size of each query's input by traversing the
+ // logical plan and adding up the sizes of all tables that appear in the plan.
+ val queryRelations = scala.collection.mutable.HashSet[String]()
+ cometSpark.sql(queryString).queryExecution.analyzed.foreach {
+ case SubqueryAlias(alias, _: LogicalRelation) =>
+ queryRelations.add(alias.name)
+ case LogicalRelation(_, _, Some(catalogTable), _) =>
+ queryRelations.add(catalogTable.identifier.table)
+ case HiveTableRelation(tableMeta, _, _, _, _) =>
+ queryRelations.add(tableMeta.identifier.table)
+ case _ =>
+ }
+ val numRows = queryRelations.map(tableSizes.getOrElse(_, 0L)).sum
+ val benchmark = new Benchmark(benchmarkName, numRows, 2, output = output)
+ benchmark.addCase(s"$name$nameSuffix") { _ =>
+ cometSpark.sql(queryString).noop()
+ }
+ benchmark.addCase(s"$name$nameSuffix") { _ =>
+ cometSpark.sql(queryString).noop()
+ }
Review Comment:
Thanks. I will remove the dupes.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#discussion_r1681816431
##########
spark/src/test/scala/org/apache/spark/sql/benchmark/CometTPCDSMicroBenchmark.scala:
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.benchmark
+
+import scala.io.Source
+
+import org.apache.spark.benchmark.Benchmark
+import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
+import org.apache.spark.sql.catalyst.plans.logical.SubqueryAlias
+import org.apache.spark.sql.execution.benchmark.TPCDSQueryBenchmark.tables
+import org.apache.spark.sql.execution.benchmark.TPCDSQueryBenchmarkArguments
+import org.apache.spark.sql.execution.datasources.LogicalRelation
+
+import org.apache.comet.CometConf
+
+/**
+ * Benchmark to measure Comet query performance with micro benchmarks that use the TPCDS data.
+ * These queries represent subsets of the full TPCDS queries.
+ *
+ * To run this benchmark:
+ * {{{
+ * // Build [tpcds-kit](https://github.com/databricks/tpcds-kit)
+ * cd /tmp && git clone https://github.com/databricks/tpcds-kit.git
+ * cd tpcds-kit/tools && make OS=MACOS
+ *
+ * // GenTPCDSData
+ * cd $COMET_HOME && mkdir /tmp/tpcds
+ * make benchmark-org.apache.spark.sql.GenTPCDSData -- --dsdgenDir /tmp/tpcds-kit/tools --location /tmp/tpcds --scaleFactor 1
+ *
+ * // CometTPCDSMicroBenchmark
+ * SPARK_GENERATE_BENCHMARK_FILES=1 make benchmark-org.apache.spark.sql.benchmark.CometTPCDSMicroBenchmark -- --data-location /tmp/tpcds
+ * }}}
+ *
+ * Results will be written to "spark/benchmarks/CometTPCDSMicroBenchmark-**results.txt".
+ */
+object CometTPCDSMicroBenchmark extends CometTPCQueryBenchmarkBase {
+
+ val queries: Seq[String] = Seq(
+ "add_many_decimals",
+ "add_many_integers",
+ "agg_high_cardinality",
+ "agg_low_cardinality",
+ "agg_sum_decimals_no_grouping",
+ "agg_sum_integers_no_grouping",
+ "case_when_column_or_null",
+ "case_when_scalar",
+ "filter_highly_selective",
+ "filter_less_selective",
+ "if_column_or_null",
+ "join_anti",
+ "join_condition",
+ "join_exploding_output",
+ "join_inner",
+ "join_left_outer",
+ "join_semi")
+
+ override def runQueries(
+ queryLocation: String,
+ queries: Seq[String],
+ tableSizes: Map[String, Long],
+ benchmarkName: String,
+ nameSuffix: String = ""): Unit = {
+ queries.foreach { name =>
+ val source = Source.fromFile(s"spark/src/test/resources/tpcds-micro-benchmarks/$name.sql")
+ val queryString = source
+ .getLines()
+ .filterNot(_.startsWith("--"))
+ .mkString("\n")
+ source.close()
+
+ // This is an indirect hack to estimate the size of each query's input by traversing the
+ // logical plan and adding up the sizes of all tables that appear in the plan.
+ val queryRelations = scala.collection.mutable.HashSet[String]()
+ cometSpark.sql(queryString).queryExecution.analyzed.foreach {
+ case SubqueryAlias(alias, _: LogicalRelation) =>
+ queryRelations.add(alias.name)
+ case LogicalRelation(_, _, Some(catalogTable), _) =>
+ queryRelations.add(catalogTable.identifier.table)
+ case HiveTableRelation(tableMeta, _, _, _, _) =>
+ queryRelations.add(tableMeta.identifier.table)
+ case _ =>
+ }
+ val numRows = queryRelations.map(tableSizes.getOrElse(_, 0L)).sum
+ val benchmark = new Benchmark(benchmarkName, numRows, 2, output = output)
+ benchmark.addCase(s"$name$nameSuffix") { _ =>
+ cometSpark.sql(queryString).noop()
+ }
+ benchmark.addCase(s"$name$nameSuffix") { _ =>
+ cometSpark.sql(queryString).noop()
+ }
Review Comment:
This is copied from the parent version of the method that is being overridden here. I assumed that these were here for a reason (maybe to allow for warmup?).
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#discussion_r1681564129
##########
spark/src/test/resources/tpcds-micro-benchmarks/add_many_decimals.sql:
##########
@@ -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.
+
+-- This is testing the cost of a complex expression that will create many intermediate arrays in Comet
Review Comment:
The comment looks not for this query?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "kazuyukitanimura (via GitHub)" <gi...@apache.org>.
kazuyukitanimura commented on code in PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#discussion_r1681836876
##########
spark/src/test/scala/org/apache/spark/sql/benchmark/CometTPCDSMicroBenchmark.scala:
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.benchmark
+
+import scala.io.Source
+
+import org.apache.spark.benchmark.Benchmark
+import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
+import org.apache.spark.sql.catalyst.plans.logical.SubqueryAlias
+import org.apache.spark.sql.execution.benchmark.TPCDSQueryBenchmark.tables
+import org.apache.spark.sql.execution.benchmark.TPCDSQueryBenchmarkArguments
+import org.apache.spark.sql.execution.datasources.LogicalRelation
+
+import org.apache.comet.CometConf
+
+/**
+ * Benchmark to measure Comet query performance with micro benchmarks that use the TPCDS data.
+ * These queries represent subsets of the full TPCDS queries.
+ *
+ * To run this benchmark:
+ * {{{
+ * // Build [tpcds-kit](https://github.com/databricks/tpcds-kit)
+ * cd /tmp && git clone https://github.com/databricks/tpcds-kit.git
+ * cd tpcds-kit/tools && make OS=MACOS
+ *
+ * // GenTPCDSData
+ * cd $COMET_HOME && mkdir /tmp/tpcds
+ * make benchmark-org.apache.spark.sql.GenTPCDSData -- --dsdgenDir /tmp/tpcds-kit/tools --location /tmp/tpcds --scaleFactor 1
+ *
+ * // CometTPCDSMicroBenchmark
+ * SPARK_GENERATE_BENCHMARK_FILES=1 make benchmark-org.apache.spark.sql.benchmark.CometTPCDSMicroBenchmark -- --data-location /tmp/tpcds
+ * }}}
+ *
+ * Results will be written to "spark/benchmarks/CometTPCDSMicroBenchmark-**results.txt".
+ */
+object CometTPCDSMicroBenchmark extends CometTPCQueryBenchmarkBase {
+
+ val queries: Seq[String] = Seq(
+ "add_many_decimals",
+ "add_many_integers",
+ "agg_high_cardinality",
+ "agg_low_cardinality",
+ "agg_sum_decimals_no_grouping",
+ "agg_sum_integers_no_grouping",
+ "case_when_column_or_null",
+ "case_when_scalar",
+ "filter_highly_selective",
+ "filter_less_selective",
+ "if_column_or_null",
+ "join_anti",
+ "join_condition",
+ "join_exploding_output",
+ "join_inner",
+ "join_left_outer",
+ "join_semi")
+
+ override def runQueries(
+ queryLocation: String,
+ queries: Seq[String],
+ tableSizes: Map[String, Long],
+ benchmarkName: String,
+ nameSuffix: String = ""): Unit = {
+ queries.foreach { name =>
+ val source = Source.fromFile(s"spark/src/test/resources/tpcds-micro-benchmarks/$name.sql")
+ val queryString = source
+ .getLines()
+ .filterNot(_.startsWith("--"))
+ .mkString("\n")
+ source.close()
+
+ // This is an indirect hack to estimate the size of each query's input by traversing the
+ // logical plan and adding up the sizes of all tables that appear in the plan.
+ val queryRelations = scala.collection.mutable.HashSet[String]()
+ cometSpark.sql(queryString).queryExecution.analyzed.foreach {
+ case SubqueryAlias(alias, _: LogicalRelation) =>
+ queryRelations.add(alias.name)
+ case LogicalRelation(_, _, Some(catalogTable), _) =>
+ queryRelations.add(catalogTable.identifier.table)
+ case HiveTableRelation(tableMeta, _, _, _, _) =>
+ queryRelations.add(tableMeta.identifier.table)
+ case _ =>
+ }
+ val numRows = queryRelations.map(tableSizes.getOrElse(_, 0L)).sum
+ val benchmark = new Benchmark(benchmarkName, numRows, 2, output = output)
+ benchmark.addCase(s"$name$nameSuffix") { _ =>
+ cometSpark.sql(queryString).noop()
+ }
+ benchmark.addCase(s"$name$nameSuffix") { _ =>
+ cometSpark.sql(queryString).noop()
+ }
Review Comment:
I just checked. This is a leftover from the early development that we were testing some feature. Eventually that was removed, and the flag was deleted, ended up the second case becoming the same as the baseline.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2232116879
> Hmmm we have microbenchmarks at https://github.com/apache/datafusion-comet/tree/main/spark/src/test/scala/org/apache/spark/sql/benchmark Wondering if we should follow the existing one.
+1 to adding this in the existing benchmarks unless running this on a cluster is a pre-requisite. It's easier to run these queries in a code profiler if we run them locally.
Also, not necessary but could we keep the sql files in a subdirectory?
Very nice selection of queries btw.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2234142009
> Can we also commit the benchmark results (as we do for the other microbenchmarks)? That way we can keep an eye open for any performance regressions.
I was under the impression that we were committing the output of the benchmarking run as well. Looks like I'm mistaken.
FWIW, Spark has a Github Actions workflow to run benchmarks (documented here: https://spark.apache.org/developer-tools.html) that allows contributors to run benchmarks from their own github fork. We could do something similar down the road.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2234221966
> Just looking at this one case, with decimal fields and only scan enabled, we are much slower. This is consistent with something I saw when working on the parallel reader.
> From a profiling run I saw that a potential bottleneck was [BosonVector.getDecimal](https://github.com/apache/datafusion-comet/blob/7ac2fb9f6672fbb29c2e5de7e62b457efc0bfebf/common/src/main/java/org/apache/comet/vector/CometVector.java#L74) which has an expensive creation of a BigInteger followed by an expensive creation of a BigDecimal.
> However, this path would be hit only for precision > 18 or if `spark.comet.use.decimal128` was set to `true` (it is `false` by default).
> Also, I'm not sure if there is a way to eliminate this though.
I was also trying to understand why this result was slower. I have created https://github.com/apache/datafusion-comet/issues/679 based on your comment so that we can use that issue to explore possible optimizations
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2232452105
> Hmmm we have microbenchmarks at https://github.com/apache/datafusion-comet/tree/main/spark/src/test/scala/org/apache/spark/sql/benchmark
>
> Wondering if we should follow the existing one.
>
> Or is this PR for submitting the jobs to clusters
I wasn't aware that these benchmarks existed 🤦
I will study them tomorrow. Thanks for pointing this out.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "kazuyukitanimura (via GitHub)" <gi...@apache.org>.
kazuyukitanimura commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2232034311
Hmmm we have microbenchmarks at https://github.com/apache/datafusion-comet/tree/main/spark/src/test/scala/org/apache/spark/sql/benchmark
Wondering if we should follow the existing one.
Or is this PR for submitting the jobs to clusters
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2234180553
> TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
> ------------------------------------------------------------------------------------------------------------------------
> add_many_decimals 20502 20648 208 14.0 71.2 1.0X
> add_many_decimals 20498 20544 65 14.1 71.2 1.0X
> add_many_decimals: Comet (Scan) 28143 28161 26 10.2 97.7 0.7X
> add_many_decimals: Comet (Scan, Exec) 19323 19497 246 14.9 67.1 1.1X
Just looking at this one case, with decimal fields and only scan enabled, we are much slower. This is consistent with something I saw when working on the parallel reader.
From a profiling run I saw that a potential bottleneck was [BosonVector.getDecimal](https://github.com/apache/datafusion-comet/blob/7ac2fb9f6672fbb29c2e5de7e62b457efc0bfebf/common/src/main/java/org/apache/comet/vector/CometVector.java#L74) which has an expensive creation of a BigInteger followed by an expensive creation of a BigDecimal.
However, this path would be hit only for precision > 18 or if `spark.comet.use.decimal128` was set to `true` (it is `false` by default).
Also, I'm not sure if there is a way to eliminate this though.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2233704546
@parthchandra @kazuyukitanimura This is ready for another review. I can now run these new microbenchmarks from the existing test framework.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2233227228
I tried integrating into the existing suite but the queries are not running correctly and I am not sure why.
Example:
```
TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
add_many_decimals 15 21 7 0.0 Infinity 1.0X
add_many_decimals 13 16 3 0.0 Infinity 1.2X
add_many_decimals: Comet (Scan) 13 15 3 0.0 Infinity 1.2X
add_many_decimals: Comet (Scan, Exec) 11 12 2 0.0 Infinity 1.4X
```
This query should take more than 20 seconds to run
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#issuecomment-2237668944
> > ## TPCDS Micro Benchmarks: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
> > add_many_decimals 20502 20648 208 14.0 71.2 1.0X
> > add_many_decimals 20498 20544 65 14.1 71.2 1.0X
> > add_many_decimals: Comet (Scan) 28143 28161 26 10.2 97.7 0.7X
> > add_many_decimals: Comet (Scan, Exec) 19323 19497 246 14.9 67.1 1.1X
>
> Just looking at this one case, with decimal fields and only scan enabled, we are much slower. This is consistent with something I saw when working on the parallel reader. From a profiling run I saw that a potential bottleneck was [BosonVector.getDecimal](https://github.com/apache/datafusion-comet/blob/7ac2fb9f6672fbb29c2e5de7e62b457efc0bfebf/common/src/main/java/org/apache/comet/vector/CometVector.java#L74) which has an expensive creation of a BigInteger followed by an expensive creation of a BigDecimal. However, this path would be hit only for precision > 18 or if `spark.comet.use.decimal128` was set to `true` (it is `false` by default). Also, I'm not sure if there is a way to eliminate this though.
For use_decimal128 is true, maybe we can skip a step in `getDecimal`: https://github.com/apache/datafusion-comet/pull/682
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Add microbenchmarks [datafusion-comet]
Posted by "kazuyukitanimura (via GitHub)" <gi...@apache.org>.
kazuyukitanimura commented on code in PR #671:
URL: https://github.com/apache/datafusion-comet/pull/671#discussion_r1681790305
##########
spark/src/test/scala/org/apache/spark/sql/benchmark/CometTPCDSMicroBenchmark.scala:
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.benchmark
+
+import scala.io.Source
+
+import org.apache.spark.benchmark.Benchmark
+import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
+import org.apache.spark.sql.catalyst.plans.logical.SubqueryAlias
+import org.apache.spark.sql.execution.benchmark.TPCDSQueryBenchmark.tables
+import org.apache.spark.sql.execution.benchmark.TPCDSQueryBenchmarkArguments
+import org.apache.spark.sql.execution.datasources.LogicalRelation
+
+import org.apache.comet.CometConf
+
+/**
+ * Benchmark to measure Comet query performance with micro benchmarks that use the TPCDS data.
+ * These queries represent subsets of the full TPCDS queries.
+ *
+ * To run this benchmark:
+ * {{{
+ * // Build [tpcds-kit](https://github.com/databricks/tpcds-kit)
+ * cd /tmp && git clone https://github.com/databricks/tpcds-kit.git
+ * cd tpcds-kit/tools && make OS=MACOS
+ *
+ * // GenTPCDSData
+ * cd $COMET_HOME && mkdir /tmp/tpcds
+ * make benchmark-org.apache.spark.sql.GenTPCDSData -- --dsdgenDir /tmp/tpcds-kit/tools --location /tmp/tpcds --scaleFactor 1
+ *
+ * // CometTPCDSMicroBenchmark
+ * SPARK_GENERATE_BENCHMARK_FILES=1 make benchmark-org.apache.spark.sql.benchmark.CometTPCDSMicroBenchmark -- --data-location /tmp/tpcds
+ * }}}
+ *
+ * Results will be written to "spark/benchmarks/CometTPCDSMicroBenchmark-**results.txt".
+ */
+object CometTPCDSMicroBenchmark extends CometTPCQueryBenchmarkBase {
+
+ val queries: Seq[String] = Seq(
+ "add_many_decimals",
+ "add_many_integers",
+ "agg_high_cardinality",
+ "agg_low_cardinality",
+ "agg_sum_decimals_no_grouping",
+ "agg_sum_integers_no_grouping",
+ "case_when_column_or_null",
+ "case_when_scalar",
+ "filter_highly_selective",
+ "filter_less_selective",
+ "if_column_or_null",
+ "join_anti",
+ "join_condition",
+ "join_exploding_output",
+ "join_inner",
+ "join_left_outer",
+ "join_semi")
+
+ override def runQueries(
+ queryLocation: String,
+ queries: Seq[String],
+ tableSizes: Map[String, Long],
+ benchmarkName: String,
+ nameSuffix: String = ""): Unit = {
+ queries.foreach { name =>
+ val source = Source.fromFile(s"spark/src/test/resources/tpcds-micro-benchmarks/$name.sql")
+ val queryString = source
+ .getLines()
+ .filterNot(_.startsWith("--"))
+ .mkString("\n")
+ source.close()
+
+ // This is an indirect hack to estimate the size of each query's input by traversing the
+ // logical plan and adding up the sizes of all tables that appear in the plan.
+ val queryRelations = scala.collection.mutable.HashSet[String]()
+ cometSpark.sql(queryString).queryExecution.analyzed.foreach {
+ case SubqueryAlias(alias, _: LogicalRelation) =>
+ queryRelations.add(alias.name)
+ case LogicalRelation(_, _, Some(catalogTable), _) =>
+ queryRelations.add(catalogTable.identifier.table)
+ case HiveTableRelation(tableMeta, _, _, _, _) =>
+ queryRelations.add(tableMeta.identifier.table)
+ case _ =>
+ }
+ val numRows = queryRelations.map(tableSizes.getOrElse(_, 0L)).sum
+ val benchmark = new Benchmark(benchmarkName, numRows, 2, output = output)
+ benchmark.addCase(s"$name$nameSuffix") { _ =>
+ cometSpark.sql(queryString).noop()
+ }
+ benchmark.addCase(s"$name$nameSuffix") { _ =>
+ cometSpark.sql(queryString).noop()
+ }
Review Comment:
Looks like these are dupes.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org