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

Remove the pageable pool #8857

Merged
merged 2 commits into from
Jul 31, 2023
Merged

Conversation

revans2
Copy link
Collaborator

@revans2 revans2 commented Jul 28, 2023

This is the first step in trying to get host memory under control. I ran performance tests and didn't see any difference in the median performance of NDA.

I didn't remove the config for the pageable pool size yet, but I am debating if I should. The original plan was to not remove it, but keep it for backwards compatibility. I thought it could be used outside of spill, but it cannot, so I think I want to adjust things with it.

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
@revans2
Copy link
Collaborator Author

revans2 commented Jul 28, 2023

build

abellina
abellina previously approved these changes Jul 28, 2023
@sameerz sameerz added the reliability Features to improve reliability or bugs that severly impact the reliability of the plugin label Jul 28, 2023
@revans2
Copy link
Collaborator Author

revans2 commented Jul 28, 2023

I adjusted the copyrights and ran NDS at scale factor 200 on my local desktop, but really configured to spill to host memory (8 GiB of GPU Memory, 32 GIB of Spill, 2 GiB of pinned, and 6x parallelism). I still need to run one more set of benchmarks to see what happens with a larger pageable pool size, but with just a 1 GiB default pool there was a 2% performance regression. I personally think that this is acceptable for now. But I will update the host memory limits to have us look at adding a pageable store back in when we are done.

@revans2
Copy link
Collaborator Author

revans2 commented Jul 28, 2023

build

@revans2
Copy link
Collaborator Author

revans2 commented Jul 31, 2023

I ran tests with pinned memory instead of pageable memory, and it the pinned memory ended up being 10% faster than the pageable memory for the same run, with spilling. The advantage of pageable is that the pinned memory must be materialized, but pageable does not. I don't think I will file a follow on issue for pageable unless we see a specific need for something like it.

@revans2 revans2 merged commit c579632 into NVIDIA:branch-23.10 Jul 31, 2023
@revans2 revans2 deleted the remove_pageable_pool branch July 31, 2023 13:34
@revans2 revans2 mentioned this pull request Jul 31, 2023
35 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reliability Features to improve reliability or bugs that severly impact the reliability of the plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants