Fix memThreshold memory tracking optimization
#18480
Merged
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
18465 optimized memory leak tracking to avoid taking a lock when the real allocation size is below `memThreshold`, but there were a few bugs in the initial implementation. For cases where the memory layer doesn't support `chpl_real_alloc_size` (`CHPL_MEM=cstdlib`) we weren't handling the unknown size sentinel so we weren't ever running the free hook, which made it look like we were leaking all memory. Fix that to run the hook if the "real size" is 0 (the unknown sentinel.) This was also broken under configurations that separately allocate arrays (like `CHPL_COMM=ugni`). For those configs we were taking a comm layer allocation and trying to ask jemalloc for the size of it, but this results in mixed allocator calls, which is undefined behavior. Fix that by making the higher level interface compute the size and passing it in instead of computing it below the level where we know what allocator memory came from. Signed-off-by: Elliot Ronaghan <ronawho@gmail.com>
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.
#18465 optimized memory leak tracking to avoid taking a lock when the real
allocation size is below
memThreshold, but there were a few bugs inthe initial implementation.
For cases where the memory layer doesn't support
chpl_real_alloc_size(
CHPL_MEM=cstdlib) we weren't handling the unknown size sentinel so weweren't ever running the free hook, which made it look like we were
leaking all memory. Fix that to run the hook if the "real size" is 0
(the unknown sentinel.)
This was also broken under configurations that separately allocate
arrays (like
CHPL_COMM=ugni). For those configs we were taking a commlayer allocation and trying to ask jemalloc for the size of it, but this
results in mixed allocator calls, which is undefined behavior. Fix that
by making the higher level interface compute the size and passing it in
instead of computing it below the level where we know what allocator
memory came from.