Repository navigation
CUDA: Reduce memory pressure from local memory tests - #8572
Merged
Merged
Conversation
Local memory usage requires a reservation of (bytes per thread) * (number of SMs) * (Max resident threads per SM), which for kernels with local memory requirements of 80KB per thread (as is the case for these tests) forces allocations into the low GBs - this is a problem on the buildfarm as multiple tests run concurrently and can result in out-of-memory conditions. This commit reduces the local memory allocations by a factor of 10 - I worry that reducing by another order of magnitude might cause local memory not to be allocated in some cases. Some copy/paste errors that slipped through in these tests are also resolved (including the name of one of the tests!).
Member
Author
|
gpuci run tests |
stuartarchibald
left a comment
stuartarchibald
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the patch. One minor thing to resolve else looks good. The main test of this will be in CI to see if the resource pressure is reduced when the tests are run in a multi-process environment against the same device(s).
stuartarchibald
added
4 - Waiting on author
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
Member
Author
|
gpuci run tests |
Member
Author
|
@stuartarchibald Many thanks for the review, suggestion committed. |
gmarkall
added
4 - Waiting on reviewer
stuartarchibald
approved these changes
Nov 4, 2022
stuartarchibald
left a comment
stuartarchibald
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the patch and fixes.
stuartarchibald
added
Pending BuildFarm
Member
|
|
Contributor
Passed. |
stuartarchibald
added
5 - Ready to merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Local memory usage requires a reservation of (bytes per thread) * (number of SMs) * (Max resident threads per SM), which for kernels with local memory requirements of 80KB per thread (as is the case for these tests) forces allocations into the low GBs - this is a problem on the buildfarm as multiple tests run concurrently and can result in out-of-memory conditions.
This commit reduces the local memory allocations by a factor of 10 - I worry that reducing by another order of magnitude might cause local memory not to be allocated in some cases.
Some copy/paste errors that slipped through in these tests are also resolved (including the name of one of the tests!).