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

[VL] [Test] Test rebasing upstream velox 11_8 #3642

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Conversation

JkSelf
Copy link
Contributor

@JkSelf JkSelf commented Nov 8, 2023

What changes were proposed in this pull request?

In the upstream Velox, it is ensured that the sorting key is unique in TopNode here. Therefore, we add a check for this in this PR.

How was this patch tested?

Pass CI.

Copy link

github-actions bot commented Nov 8, 2023

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/oap-project/gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

@JkSelf
Copy link
Contributor Author

JkSelf commented Nov 8, 2023

/Benchmark Velox

@GlutenPerfBot
Copy link
Contributor

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

query log/native_3642_time.csv log/native_master_11_07_2023_e3eff1d8f_time.csv difference percentage
q1 34.53 34.38 -0.149 99.57%
q2 24.72 25.03 0.309 101.25%
q3 38.61 38.14 -0.464 98.80%
q4 37.26 37.57 0.312 100.84%
q5 69.90 71.50 1.599 102.29%
q6 6.39 6.26 -0.126 98.03%
q7 86.78 82.22 -4.553 94.75%
q8 90.09 86.95 -3.143 96.51%
q9 121.62 119.81 -1.814 98.51%
q10 53.04 51.26 -1.780 96.64%
q11 20.53 19.73 -0.796 96.12%
q12 25.89 24.39 -1.498 94.21%
q13 49.19 50.30 1.104 102.24%
q14 17.91 17.67 -0.242 98.65%
q15 31.57 30.35 -1.212 96.16%
q16 16.25 16.20 -0.053 99.67%
q17 101.45 101.51 0.059 100.06%
q18 148.22 148.26 0.038 100.03%
q19 16.59 16.17 -0.420 97.47%
q20 29.62 30.31 0.688 102.32%
q21 226.25 224.88 -1.365 99.40%
q22 13.44 14.08 0.646 104.80%
total 1259.84 1246.98 -12.863 98.98%

@JkSelf JkSelf force-pushed the test_11_08 branch 3 times, most recently from 6a27cf3 to dfa86bd Compare November 8, 2023 08:57
Copy link
Contributor

@rui-mo rui-mo left a comment

Choose a reason for hiding this comment

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

How many tets are affected by Velox's change? If there are many of them, maybe we should consider removing duplicated keys in Gluten before sending to Velox.

@JkSelf
Copy link
Contributor Author

JkSelf commented Nov 8, 2023

How many tets are affected by Velox's change? If there are many of them, maybe we should consider removing duplicated keys in Gluten before sending to Velox.

@rui-mo Only Q72 in TPC-DS is affected. Maybe we can remove the duplicated keys in the follow PRs.

@rui-mo
Copy link
Contributor

rui-mo commented Nov 9, 2023

How does its fallback affect the performance of q72?

Copy link
Contributor

@zhouyuan zhouyuan left a comment

Choose a reason for hiding this comment

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

👍

@JkSelf JkSelf merged commit 2bb522f into apache:main Nov 9, 2023
16 checks passed
@JkSelf
Copy link
Contributor Author

JkSelf commented Nov 9, 2023

How does its fallback affect the performance of q72?

@rui-mo There is no performance impact.

@GlutenPerfBot
Copy link
Contributor

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

query log/native_3642_time.csv log/native_master_11_08_2023_29b58997a_time.csv difference percentage
q1 32.99 34.11 1.124 103.41%
q2 22.98 25.13 2.150 109.36%
q3 38.64 38.62 -0.023 99.94%
q4 37.28 36.07 -1.204 96.77%
q5 71.86 71.84 -0.013 99.98%
q6 6.36 7.59 1.234 119.41%
q7 84.69 85.82 1.130 101.33%
q8 87.29 86.97 -0.323 99.63%
q9 120.32 121.86 1.543 101.28%
q10 54.33 54.80 0.476 100.88%
q11 19.88 20.59 0.707 103.56%
q12 24.75 24.63 -0.116 99.53%
q13 48.39 49.95 1.565 103.23%
q14 18.15 16.99 -1.153 93.65%
q15 30.88 30.97 0.093 100.30%
q16 16.22 16.51 0.289 101.78%
q17 103.34 102.40 -0.944 99.09%
q18 148.73 148.28 -0.457 99.69%
q19 15.27 15.13 -0.131 99.14%
q20 29.65 29.97 0.325 101.10%
q21 226.71 222.20 -4.508 98.01%
q22 13.06 13.55 0.489 103.74%
total 1251.75 1254.01 2.252 100.18%

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.

4 participants