GTEST/UCT: Limit MEMIC allocation to prevent out-of-memory test failures #9892
Conversation
This file contains 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
yosefe
reviewed
May 21, 2024
test/gtest/uct/test_md.cc
Outdated
| @@ -411,7 +411,8 @@ UCS_TEST_SKIP_COND_P(test_md, alloc, | |||
| ucs_for_each_bit(mem_type, md_attr().alloc_mem_types) { | |||
| for (unsigned i = 0; i < iterations; ++i) { | |||
| size = orig_size = ucs::rand() % | |||
| ucs_min(65536, md_attr().max_alloc); | |||
| ucs_min(65536, | |||
| ucs_max(md_attr().max_alloc / 8, 1024)); | |||
There was a problem hiding this comment.
- if 1024 > max_alloc this can allocate too much
- simplify using helper vars
There was a problem hiding this comment.
re (1) - see line 416 below. no need to add extra checks, it's already there..
gleon99
force-pushed
the
lgenkin/silence-memic-failures
branch
from
f72c0c7
to
833bfa4
Compare
yosefe
reviewed
May 21, 2024
test/gtest/uct/test_md.cc
Outdated
| @@ -384,6 +384,7 @@ UCS_TEST_SKIP_COND_P(test_md, alloc, | |||
| uct_md_h md_ref = md(); | |||
| uct_alloc_method_t method = UCT_ALLOC_METHOD_MD; | |||
| unsigned num_alloc_failures = 0; | |||
| const auto max_alloc = ucs_max(md_attr().max_alloc / 8, 1024); | |||
There was a problem hiding this comment.
if max_alloc < 1024, we may allocate 1024 which is wrong
it should be
const auto max_alloc = md_attr().max_alloc;
const auto max_size = ucs_max(max_alloc / 8, ucs_min(max_alloc, 1024))
There was a problem hiding this comment.
I think line 417 prevents it. Anyway, I'll refactor a bit. Push shortly.
|
@yosefe I made some refactoring to remove excessive ops in each iter. wdyt? |
yosefe
reviewed
May 22, 2024
test/gtest/uct/test_md.cc
Outdated
| @@ -408,12 +412,13 @@ UCS_TEST_SKIP_COND_P(test_md, alloc, | |||
| pack_params.flags = UCT_MD_MKEY_PACK_FLAG_INVALIDATE_RMA | | |||
| UCT_MD_MKEY_PACK_FLAG_INVALIDATE_AMO; | |||
|
|
|||
| max_size = ucs_min(max_size, 63356); | |||
| max_size = ucs_rounddown_pow2(max_size); | |||
There was a problem hiding this comment.
maybe do ucs_align_down_pow2(max_size, sizeof(size_t)) ?
yosefe
approved these changes
May 22, 2024
yosefe
requested changes
May 22, 2024
| @@ -8,6 +8,12 @@ | |||
|
|
|||
| class uct_atomic_key_reg_rdma_mem_type : public uct_amo_test { | |||
| protected: | |||
| void init() override | |||
| { | |||
| modify_config("DM_COUNT", 0); | |||
There was a problem hiding this comment.
IMO it should be IB_DM_COUNT
pls validate it really works..
yosefe
approved these changes
May 23, 2024
gleon99
force-pushed
the
lgenkin/silence-memic-failures
branch
from
7198c61
to
d7d57e1
Compare
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.
What
Cap MEMIC allocations, to reduce out-of-memory failures
Why ?
Memory is very limited, and some might already be allocated.