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-5731][CORE] Fix the logic to calculate rss shuffle write time #5742

Merged
merged 1 commit into from
May 15, 2024

Conversation

hahazyb201
Copy link
Contributor

What changes were proposed in this pull request?

Change the logic to calculate the shuffle write time. Shuffle write time is currently composed of splitResult.getTotalWriteTime + splitResult.getTotalPushTime
And TotalWriteTime in the current implementation is added twice into the final shuffle write time metric.

(Fixes: #5731)

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
E2e manual test.

Copy link

#5731

@FelixYBW FelixYBW requested a review from marin-ma May 14, 2024 17:01
@marin-ma marin-ma changed the title [GLUTEN-5731][CORE] Fix the logic to calculate shuffle write time [GLUTEN-5731][CORE] Fix the logic to calculate rss shuffle write time May 15, 2024
@marin-ma
Copy link
Contributor

@kerwin-zk Could you help to confirm this change? Thanks!

Copy link
Contributor

@kerwin-zk kerwin-zk left a comment

Choose a reason for hiding this comment

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

LGTM.Thanks!

@marin-ma marin-ma merged commit 2984bd7 into apache:main May 15, 2024
42 checks passed
@GlutenPerfBot
Copy link
Contributor

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

query log/native_master_05_15_2024_time.csv log/native_master_05_14_2024_c7306128a_time.csv difference percentage
q1 18.13 15.68 -2.452 86.48%
q2 14.70 17.96 3.264 122.21%
q3 4.30 2.26 -2.044 52.50%
q4 61.79 63.27 1.478 102.39%
q5 7.96 7.96 0.003 100.03%
q6 3.75 4.53 0.786 120.99%
q7 5.67 4.13 -1.531 72.97%
q8 3.77 5.99 2.214 158.72%
q9 18.07 15.83 -2.240 87.60%
q10 9.46 9.96 0.501 105.30%
q11 34.82 38.15 3.324 109.55%
q12 2.40 1.33 -1.069 55.37%
q13 5.58 7.02 1.435 125.72%
q14a 44.17 42.18 -1.987 95.50%
q14b 43.23 42.14 -1.086 97.49%
q15 2.47 2.53 0.052 102.11%
q16 39.90 39.20 -0.701 98.24%
q17 7.15 4.81 -2.335 67.35%
q18 5.97 7.02 1.050 117.59%
q19 2.18 3.63 1.449 166.32%
q20 2.68 1.32 -1.364 49.15%
q21 1.00 5.87 4.870 587.02%
q22 7.87 8.63 0.754 109.57%
q23a 81.08 82.70 1.622 102.00%
q23b 99.94 101.96 2.020 102.02%
q24a 77.77 71.15 -6.621 91.49%
q24b 69.48 81.87 12.395 117.84%
q25 4.31 4.34 0.021 100.48%
q26 2.63 2.53 -0.101 96.17%
q27 4.71 3.02 -1.693 64.06%
q28 21.78 19.99 -1.792 91.77%
q29 8.32 9.60 1.273 115.30%
q30 5.58 4.16 -1.416 74.61%
q31 5.78 6.89 1.114 119.28%
q32 1.10 1.08 -0.018 98.39%
q33 5.73 5.69 -0.041 99.28%
q34 5.46 6.22 0.767 114.05%
q35 6.64 6.57 -0.075 98.86%
q36 3.11 2.92 -0.197 93.67%
q37 3.84 5.19 1.342 134.93%
q38 11.79 15.67 3.872 132.83%
q39a 3.27 5.44 2.171 166.38%
q39b 3.29 3.80 0.510 115.46%
q40 3.61 3.86 0.251 106.96%
q41 0.55 0.58 0.033 106.07%
q42 0.83 0.99 0.166 120.12%
q43 3.59 3.59 0.006 100.18%
q44 7.10 6.83 -0.263 96.29%
q45 3.53 5.29 1.758 149.79%
q46 3.11 3.01 -0.102 96.74%
q47 17.94 14.14 -3.799 78.82%
q48 4.84 4.51 -0.328 93.22%
q49 7.39 7.00 -0.393 94.68%
q50 30.92 23.39 -7.529 75.65%
q51 8.59 8.36 -0.232 97.30%
q52 1.12 0.91 -0.211 81.19%
q53 1.75 1.72 -0.022 98.72%
q54 3.11 3.09 -0.027 99.14%
q55 1.04 0.97 -0.071 93.13%
q56 5.35 5.36 0.016 100.30%
q57 8.49 8.35 -0.139 98.37%
q58 2.39 2.44 0.049 102.04%
q59 15.43 16.90 1.476 109.57%
q60 5.87 6.04 0.175 102.98%
q61 8.72 6.33 -2.386 72.64%
q62 3.90 4.18 0.278 107.15%
q63 1.93 1.96 0.029 101.52%
q64 48.79 48.90 0.109 100.22%
q65 13.50 13.69 0.192 101.42%
q66 2.95 3.09 0.141 104.78%
q67 350.79 353.63 2.848 100.81%
q68 4.02 3.67 -0.345 91.41%
q69 7.71 12.38 4.663 160.45%
q70 8.44 7.43 -1.005 88.09%
q71 4.29 2.18 -2.112 50.76%
q72 189.56 188.50 -1.057 99.44%
q73 2.15 2.15 0.002 100.10%
q74 20.93 21.01 0.078 100.37%
q75 25.12 24.75 -0.363 98.55%
q76 8.64 8.26 -0.378 95.63%
q77 1.76 1.80 0.038 102.14%
q78 38.03 38.36 0.329 100.87%
q79 3.47 3.33 -0.140 95.96%
q80 11.54 11.09 -0.442 96.17%
q81 5.67 5.42 -0.249 95.61%
q82 6.27 6.17 -0.097 98.45%
q83 1.56 1.41 -0.156 90.04%
q84 2.63 3.11 0.481 118.34%
q85 7.92 6.65 -1.271 83.97%
q86 2.95 3.04 0.090 103.04%
q87 13.32 11.94 -1.382 89.62%
q88 16.55 16.37 -0.174 98.95%
q89 2.74 5.14 2.405 187.87%
q90 6.45 6.11 -0.344 94.67%
q91 2.60 2.66 0.065 102.51%
q92 1.26 1.03 -0.227 81.97%
q93 28.79 28.99 0.208 100.72%
q94 21.15 21.04 -0.110 99.48%
q9 86.57 84.20 -2.371 97.26%
q5 2.51 2.52 0.013 100.51%
q96 12.37 12.50 0.130 101.05%
q97 1.86 1.90 0.040 102.17%
q98 8.93 8.27 -0.660 92.61%
q99 8.93 8.27 -0.660 92.61%
total 1897.48 1904.70 7.212 100.38%

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

Successfully merging this pull request may close these issues.

Shuffle write time metric is wrong
4 participants