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-8327][CORE][Part-3] Introduce the ConfigEntry to gluten config #8431

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

yikf
Copy link
Contributor

@yikf yikf commented Jan 6, 2025

What changes were proposed in this pull request?

This PR is the part-3 of the revert commit back and is used to introduce the ConfigEntry to gluten config.

How was this patch tested?

GA.

@github-actions github-actions bot added the CORE works for Gluten Core label Jan 6, 2025
Copy link

github-actions bot commented Jan 6, 2025

#8327

@yikf yikf changed the title [GLUTEN-8327][CORE][Part-3] Introduce the `ConfigEntry to gluten config [GLUTEN-8327][CORE][Part-3] Introduce the ConfigEntry to gluten config Jan 6, 2025
Copy link

github-actions bot commented Jan 6, 2025

Run Gluten Clickhouse CI on x86

Copy link

github-actions bot commented Jan 6, 2025

Run Gluten Clickhouse CI on x86

Copy link

github-actions bot commented Jan 6, 2025

Run Gluten Clickhouse CI on x86

Copy link

github-actions bot commented Jan 6, 2025

Run Gluten Clickhouse CI on x86

Copy link

github-actions bot commented Jan 6, 2025

Run Gluten Clickhouse CI on x86

1 similar comment
Copy link

github-actions bot commented Jan 6, 2025

Run Gluten Clickhouse CI on x86

@zhztheplayer
Copy link
Member

@yikf Thanks for bringing the code back. Is there any performance-related change you wanted to highlight between this one and the original PR?

@yikf
Copy link
Contributor Author

yikf commented Jan 7, 2025

@yikf Thanks for bringing the code back. Is there any performance-related change you wanted to highlight between this one and the original PR?

yes, after splitting the original PR, we identified the issue as being related to the current PR's changes. after offline discussion and investigation with @baibaichen , it was determined that the problem was caused by the modification. Now, we have avoided synchronization and copying.

Last night, @baibaichen verified this PR and there are no performance issues now.

@yikf
Copy link
Contributor Author

yikf commented Jan 7, 2025

@zhztheplayer BTW, Please don't merge this PR for the time being. Regarding the introduction of the custom ConfigEntry, @baibaichen have another idea. We can discuss it.

@baibaichen
Copy link
Contributor

With this commit 8bf2666, the performance issue is fixed, see below image:
8f9b1c70bd1795da2282a23aa2c190c

Copy link

github-actions bot commented Jan 7, 2025

Run Gluten Clickhouse CI on x86

Copy link
Contributor

@jackylee-ch jackylee-ch left a comment

Choose a reason for hiding this comment

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

👍

@baibaichen baibaichen merged commit a5d1ea6 into apache:main Jan 7, 2025
48 checks passed
@GlutenPerfBot
Copy link
Contributor

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

query log/native_master_01_07_2025_time.csv log/native_master_01_07_2025_fe02feefb8_time.csv difference percentage
q1 16.78 16.14 -0.642 96.17%
q2 13.94 17.02 3.083 122.11%
q3 5.68 2.90 -2.787 50.97%
q4 86.46 85.45 -1.014 98.83%
q5 13.28 12.79 -0.487 96.33%
q6 5.37 4.60 -0.769 85.68%
q7 6.06 6.73 0.673 111.10%
q8 6.44 4.81 -1.629 74.71%
q9 27.65 28.44 0.789 102.85%
q10 13.31 12.79 -0.524 96.07%
q11 42.27 42.52 0.248 100.59%
q12 2.02 1.96 -0.059 97.09%
q13 9.13 8.25 -0.878 90.38%
q14a 62.24 61.76 -0.478 99.23%
q14b 57.53 58.31 0.783 101.36%
q15 3.76 3.32 -0.448 88.10%
q16 29.08 29.25 0.172 100.59%
q17 7.57 7.73 0.155 102.05%
q18 9.63 10.09 0.461 104.78%
q19 3.65 3.68 0.027 100.74%
q20 2.42 2.39 -0.035 98.57%
q21 1.59 1.50 -0.091 94.29%
q22 9.66 9.23 -0.423 95.62%
q23a 135.80 135.52 -0.282 99.79%
q23b 162.60 163.30 0.698 100.43%
q24a 98.24 105.68 7.445 107.58%
q24b 94.68 96.70 2.017 102.13%
q25 6.55 6.50 -0.050 99.23%
q26 4.57 3.98 -0.588 87.14%
q27 6.64 4.58 -2.065 68.91%
q28 38.71 37.08 -1.637 95.77%
q29 19.24 17.48 -1.761 90.84%
q30 6.35 6.63 0.277 104.36%
q31 10.31 10.65 0.349 103.39%
q32 2.33 2.08 -0.256 89.05%
q33 8.42 7.52 -0.896 89.36%
q34 4.06 4.27 0.211 105.20%
q35 12.11 12.20 0.086 100.71%
q36 5.53 5.41 -0.120 97.83%
q37 5.18 5.32 0.138 102.67%
q38 17.21 18.16 0.954 105.54%
q39a 5.15 4.47 -0.684 86.71%
q39b 4.84 5.05 0.204 104.21%
q40 5.35 6.29 0.939 117.54%
q41 0.90 0.89 -0.015 98.29%
q42 1.32 1.51 0.191 114.47%
q43 4.50 4.50 -0.000 100.00%
q44 12.03 12.47 0.447 103.72%
q45 4.35 4.19 -0.164 96.24%
q46 5.28 5.78 0.499 109.45%
q47 20.11 20.34 0.223 101.11%
q48 6.29 6.78 0.491 107.82%
q49 10.94 11.00 0.064 100.58%
q50 40.80 38.42 -2.381 94.16%
q51 14.63 14.67 0.040 100.27%
q52 1.92 1.38 -0.544 71.70%
q53 3.05 3.31 0.259 108.50%
q54 7.31 6.80 -0.512 92.99%
q55 1.34 1.28 -0.056 95.85%
q56 7.24 7.13 -0.109 98.50%
q57 12.25 13.29 1.044 108.52%
q58 3.45 3.43 -0.017 99.51%
q59 6.36 7.14 0.778 112.23%
q60 9.09 8.96 -0.126 98.62%
q61 8.66 7.73 -0.929 89.27%
q62 5.06 5.09 0.034 100.68%
q63 3.01 3.20 0.185 106.16%
q64 63.67 62.56 -1.112 98.25%
q65 29.05 29.84 0.784 102.70%
q66 4.32 4.31 -0.010 99.78%
q67 229.84 228.28 -1.566 99.32%
q68 5.31 4.47 -0.847 84.07%
q69 7.14 7.78 0.641 108.98%
q70 11.98 13.06 1.077 108.99%
q71 5.92 6.04 0.115 101.95%
q72 39.80 38.45 -1.346 96.62%
q73 3.17 3.45 0.282 108.89%
q74 26.64 26.70 0.062 100.23%
q75 43.95 43.85 -0.099 99.77%
q76 14.97 14.17 -0.801 94.65%
q77 3.50 4.13 0.626 117.86%
q78 84.27 84.42 0.146 100.17%
q79 5.13 5.28 0.145 102.83%
q80 16.67 16.95 0.288 101.73%
q81 8.33 8.49 0.159 101.91%
q82 10.07 10.13 0.065 100.65%
q83 2.79 2.58 -0.203 92.71%
q84 4.03 3.59 -0.444 89.00%
q85 10.14 9.86 -0.280 97.24%
q86 4.66 4.57 -0.096 97.95%
q87 17.74 18.85 1.111 106.26%
q88 23.86 23.24 -0.623 97.39%
q89 4.43 4.68 0.243 105.48%
q90 3.20 3.30 0.099 103.08%
q91 5.24 5.51 0.274 105.24%
q92 2.20 2.15 -0.058 97.37%
q93 54.78 54.10 -0.684 98.75%
q94 16.92 16.93 0.014 100.08%
q9 96.04 97.68 1.646 101.71%
q5 2.93 3.01 0.083 102.85%
q96 28.05 28.58 0.523 101.87%
q97 2.77 2.75 -0.013 99.54%
q98 10.39 10.63 0.242 102.33%
q99 10.39 10.63 0.242 102.33%
total 2183.21 2184.16 0.953 100.04%

@yikf yikf deleted the configEntry branch January 8, 2025 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CORE works for Gluten Core VELOX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants