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] In OAP Velox fork, simplify shrinking/spilling code for Velox memory pool #3586

Merged
merged 14 commits into from
Nov 2, 2023

Conversation

zhztheplayer
Copy link
Member

@zhztheplayer zhztheplayer commented Nov 1, 2023

I've done some work to minimize the for-pick PR facebookincubator/velox#5790, the change included in this patch would be needed for Gluten then.

@apache apache deleted a comment from github-actions bot Nov 1, 2023
Copy link

github-actions bot commented Nov 1, 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:

@zhztheplayer zhztheplayer changed the title [VL] Simplify shrinking/spilling code on Velox memory pool [VL] Simplify shrinking/spilling code for Velox memory pool Nov 1, 2023
@zhztheplayer
Copy link
Member Author

/Benchmark Velox

@GlutenPerfBot
Copy link
Contributor

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

query log/native_3586_time.csv log/native_master_11_01_2023_697b2bda2_time.csv difference percentage
q1 34.57 34.18 -0.390 98.87%
q2 23.10 24.84 1.739 107.53%
q3 40.63 40.06 -0.565 98.61%
q4 36.70 37.29 0.589 101.60%
q5 71.38 71.36 -0.020 99.97%
q6 8.71 8.95 0.234 102.68%
q7 87.60 86.23 -1.368 98.44%
q8 88.11 84.56 -3.549 95.97%
q9 120.69 121.30 0.609 100.50%
q10 52.51 50.93 -1.579 96.99%
q11 20.35 20.30 -0.049 99.76%
q12 27.50 26.50 -1.000 96.36%
q13 49.80 48.85 -0.949 98.09%
q14 19.42 18.13 -1.289 93.36%
q15 33.86 35.06 1.204 103.56%
q16 16.40 16.16 -0.238 98.55%
q17 101.94 102.07 0.130 100.13%
q18 147.37 149.11 1.743 101.18%
q19 18.04 18.26 0.212 101.17%
q20 31.43 31.06 -0.368 98.83%
q21 225.84 223.74 -2.101 99.07%
q22 13.62 13.21 -0.419 96.93%
total 1269.58 1262.16 -7.424 99.42%

@zhztheplayer zhztheplayer changed the title [VL] Simplify shrinking/spilling code for Velox memory pool [VL] In OAP Velox fork, simplify shrinking/spilling code for Velox memory pool Nov 2, 2023
@@ -62,26 +63,28 @@ class ListenableArbitrator : public velox::memory::MemoryArbitrator {
}

void reserveMemory(velox::memory::MemoryPool* pool, uint64_t) override {
growPool(pool, memoryPoolInitCapacity_);
std::lock_guard<std::recursive_mutex> l(mutex_);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍
could we remove AllocationListener's mutex?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's keep that mutex for now since this one only works in Velox backend.

@zhztheplayer zhztheplayer marked this pull request as ready for review November 2, 2023 04:39
@zhztheplayer zhztheplayer merged commit 27defd8 into apache:main Nov 2, 2023
5 of 15 checks passed
@GlutenPerfBot
Copy link
Contributor

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

query log/native_3586_time.csv log/native_master_11_01_2023_697b2bda2_time.csv difference percentage
q1 36.37 34.18 -2.197 93.96%
q2 24.67 24.84 0.177 100.72%
q3 41.00 40.06 -0.940 97.71%
q4 36.87 37.29 0.418 101.13%
q5 72.29 71.36 -0.929 98.71%
q6 8.60 8.95 0.343 103.99%
q7 88.00 86.23 -1.767 97.99%
q8 87.15 84.56 -2.593 97.02%
q9 120.53 121.30 0.771 100.64%
q10 52.58 50.93 -1.646 96.87%
q11 19.96 20.30 0.340 101.70%
q12 26.26 26.50 0.248 100.94%
q13 48.76 48.85 0.091 100.19%
q14 17.95 18.13 0.186 101.03%
q15 35.20 35.06 -0.131 99.63%
q16 16.37 16.16 -0.206 98.74%
q17 101.08 102.07 0.988 100.98%
q18 148.09 149.11 1.019 100.69%
q19 16.91 18.26 1.342 107.94%
q20 30.90 31.06 0.154 100.50%
q21 225.06 223.74 -1.318 99.41%
q22 13.31 13.21 -0.101 99.24%
total 1267.91 1262.16 -5.752 99.55%

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