Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GLUTEN-8479][CORE][Part-1] Remove unnecessary config #8480

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

yikf
Copy link
Contributor

@yikf yikf commented Jan 9, 2025

What changes were proposed in this pull request?

Fix #8479, This PR removed some unused configurations and methods for getting configurations. The removed configurations are as follows:

spark.gluten.sql.columnar.columnarToRow
spark.gluten.sql.columnar.oneRowRelation
spark.gluten.sql.columnar.logicalJoinOptimizationLevel

How was this patch tested?

GA.

Copy link

github-actions bot commented Jan 9, 2025

#8479

Copy link

github-actions bot commented Jan 9, 2025

Run Gluten ClickHouse CI on ARM

@yikf
Copy link
Contributor Author

yikf commented Jan 9, 2025

@jackylee-ch @baibaichen Could you please take a look, thanks!

Copy link

github-actions bot commented Jan 9, 2025

Run Gluten ClickHouse CI on ARM

1 similar comment
Copy link

github-actions bot commented Jan 9, 2025

Run Gluten ClickHouse CI on ARM

@yikf yikf force-pushed the unnecessary-config branch from 8e3c174 to a1bc64a Compare January 9, 2025 06:01
Copy link

github-actions bot commented Jan 9, 2025

Run Gluten ClickHouse CI on ARM

@yikf yikf force-pushed the unnecessary-config branch from a1bc64a to c28ccef Compare January 9, 2025 06:08
@github-actions github-actions bot removed the VELOX label Jan 9, 2025
Copy link

github-actions bot commented Jan 9, 2025

Run Gluten ClickHouse CI on ARM

1 similar comment
Copy link

github-actions bot commented Jan 9, 2025

Run Gluten ClickHouse CI on ARM

@Yohahaha
Copy link
Contributor

please add the removed configs in PR description.

@yikf
Copy link
Contributor Author

yikf commented Jan 13, 2025

please add the removed configs in PR description.

added, thanks!

Copy link

Run Gluten ClickHouse CI on ARM

Copy link
Contributor

@Yohahaha Yohahaha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! left one comment.

@yikf yikf force-pushed the unnecessary-config branch from 533132b to 65543fc Compare January 14, 2025 02:30
Copy link

Run Gluten ClickHouse CI on ARM

Copy link

Run Gluten ClickHouse CI on ARM

@jackylee-ch
Copy link
Contributor

Run Gluten Clickhouse CI on x86

@@ -93,7 +93,6 @@ object RunTPCHTest {
.config("spark.databricks.delta.snapshotPartitions", 1)
.config("spark.databricks.delta.properties.defaults.checkpointInterval", 5)
.config("spark.databricks.delta.stalenessLimit", 3600 * 1000)
.config("spark.gluten.sql.columnar.columnarToRow", columnarColumnToRow)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Could remove the variable definition if unused?

Copy link
Contributor

@jackylee-ch jackylee-ch Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They has be removed in this pr. see GlutenConfig change

@jackylee-ch jackylee-ch merged commit eec5461 into apache:main Jan 15, 2025
48 checks passed
@yikf yikf deleted the unnecessary-config branch January 15, 2025 11:12
@GlutenPerfBot
Copy link
Contributor

===== Performance report for TPCDS SF2000 with Velox backend, for reference only ====

query log/native_master_01_15_2025_time.csv log/native_master_01_14_2025_ea58aab734_time.csv difference percentage
q1 16.25 15.96 -0.292 98.20%
q2 16.73 15.23 -1.494 91.07%
q3 5.05 4.78 -0.266 94.73%
q4 85.08 86.18 1.099 101.29%
q5 11.49 11.33 -0.163 98.58%
q6 5.32 5.09 -0.230 95.67%
q7 7.11 6.44 -0.667 90.62%
q8 6.83 4.99 -1.833 73.15%
q9 27.61 29.20 1.594 105.77%
q10 13.99 12.26 -1.735 87.60%
q11 43.43 41.87 -1.560 96.41%
q12 2.28 1.82 -0.461 79.78%
q13 12.26 9.43 -2.835 76.88%
q14a 67.47 66.22 -1.246 98.15%
q14b 56.78 57.69 0.914 101.61%
q15 3.50 3.47 -0.030 99.15%
q16 29.51 28.99 -0.514 98.26%
q17 7.46 7.63 0.169 102.27%
q18 10.36 10.10 -0.264 97.45%
q19 3.66 4.12 0.462 112.65%
q20 2.26 2.31 0.045 102.00%
q21 1.61 1.86 0.249 115.47%
q22 9.85 9.60 -0.253 97.43%
q23a 138.58 135.90 -2.680 98.07%
q23b 164.35 162.60 -1.746 98.94%
q24a 106.72 99.90 -6.825 93.60%
q24b 99.73 99.21 -0.520 99.48%
q25 8.21 7.71 -0.500 93.90%
q26 6.13 4.97 -1.160 81.08%
q27 5.92 4.91 -1.008 82.97%
q28 38.18 37.90 -0.288 99.24%
q29 18.68 19.90 1.225 106.56%
q30 7.75 6.53 -1.221 84.24%
q31 11.95 10.13 -1.817 84.79%
q32 2.26 2.18 -0.084 96.29%
q33 7.49 7.71 0.215 102.87%
q34 4.79 5.79 0.999 120.87%
q35 10.48 12.06 1.577 115.05%
q36 5.49 6.22 0.730 113.30%
q37 5.25 5.20 -0.048 99.08%
q38 17.51 25.34 7.828 144.69%
q39a 6.13 4.43 -1.697 72.33%
q39b 5.84 4.95 -0.891 84.73%
q40 5.72 6.85 1.129 119.73%
q41 0.86 0.89 0.029 103.42%
q42 1.27 2.06 0.789 162.31%
q43 4.57 4.55 -0.021 99.54%
q44 11.30 12.39 1.091 109.66%
q45 4.35 4.16 -0.191 95.62%
q46 5.56 5.49 -0.069 98.77%
q47 19.29 19.43 0.149 100.77%
q48 6.26 6.56 0.301 104.81%
q49 11.57 10.41 -1.159 89.99%
q50 38.83 41.21 2.379 106.13%
q51 14.02 14.48 0.464 103.31%
q52 1.15 1.20 0.048 104.12%
q53 2.96 3.04 0.079 102.65%
q54 7.07 6.79 -0.270 96.18%
q55 1.38 2.17 0.791 157.38%
q56 7.25 7.28 0.033 100.45%
q57 13.40 12.65 -0.747 94.42%
q58 3.54 3.29 -0.259 92.71%
q59 7.33 6.34 -0.990 86.49%
q60 7.89 7.85 -0.039 99.50%
q61 8.24 7.61 -0.631 92.34%
q62 5.40 5.51 0.110 102.04%
q63 3.35 3.28 -0.077 97.69%
q64 65.24 62.43 -2.814 95.69%
q65 30.98 30.42 -0.555 98.21%
q66 4.59 4.11 -0.483 89.47%
q67 227.80 230.50 2.700 101.19%
q68 5.45 4.25 -1.200 78.00%
q69 7.76 7.06 -0.702 90.96%
q70 12.63 12.54 -0.095 99.24%
q71 5.73 5.40 -0.334 94.18%
q72 39.54 39.65 0.113 100.29%
q73 3.36 3.31 -0.047 98.61%
q74 29.78 26.32 -3.464 88.37%
q75 42.98 44.39 1.407 103.27%
q76 14.36 14.63 0.270 101.88%
q77 3.46 3.91 0.453 113.10%
q78 82.97 85.13 2.158 102.60%
q79 5.16 5.13 -0.024 99.53%
q80 17.04 17.33 0.293 101.72%
q81 8.57 7.98 -0.595 93.05%
q82 10.78 10.47 -0.311 97.12%
q83 2.59 3.06 0.470 118.17%
q84 3.50 3.70 0.204 105.84%
q85 10.17 10.04 -0.126 98.76%
q86 5.17 4.52 -0.657 87.30%
q87 18.11 17.97 -0.141 99.22%
q88 23.54 24.37 0.829 103.52%
q89 4.86 4.42 -0.440 90.96%
q90 3.58 3.35 -0.231 93.55%
q91 7.21 6.67 -0.543 92.47%
q92 2.16 2.21 0.043 101.99%
q93 55.10 51.40 -3.703 93.28%
q94 19.29 17.54 -1.752 90.92%
q9 97.23 97.12 -0.117 99.88%
q5 3.33 3.17 -0.164 95.08%
q96 28.57 28.04 -0.529 98.15%
q97 3.23 2.80 -0.426 86.79%
q98 10.26 9.81 -0.444 95.67%
q99 10.26 9.81 -0.444 95.67%
total 2225.98 2200.74 -25.236 98.87%

@GlutenPerfBot
Copy link
Contributor

===== Performance report for TPCH SF2000 with Velox backend, for reference only ====

query log/native_master_01_15_2025_time.csv log/native_master_01_14_2025_ea58aab734_time.csv difference percentage
q1 42.66 44.87 2.210 105.18%
q2 44.58 44.60 0.027 100.06%
q3 92.50 91.61 -0.895 99.03%
q4 70.55 70.24 -0.303 99.57%
q5 178.53 179.79 1.256 100.70%
q6 19.53 18.83 -0.699 96.42%
q7 106.19 104.68 -1.510 98.58%
q8 183.92 184.88 0.964 100.52%
q9 278.70 277.97 -0.730 99.74%
q10 101.00 102.77 1.767 101.75%
q11 34.66 34.15 -0.508 98.53%
q12 42.73 43.07 0.335 100.78%
q13 75.86 78.62 2.758 103.63%
q14 35.65 36.60 0.951 102.67%
q15 66.13 66.30 0.167 100.25%
q16 26.57 26.99 0.422 101.59%
q17 232.94 232.07 -0.863 99.63%
q18 356.07 365.90 9.831 102.76%
q19 36.80 37.35 0.548 101.49%
q20 58.37 59.66 1.289 102.21%
q21 533.08 530.94 -2.136 99.60%
q22 25.15 24.77 -0.382 98.48%
total 2642.17 2656.66 14.496 100.55%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CORE] Split the configuration into individual modules that belong to each other.
5 participants