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

[BUG] arena_memory_resource allows creation with a maximum allocation size smaller than its minimum superblock size #919

Closed
harrism opened this issue Nov 16, 2021 · 3 comments · Fixed by #916
Assignees
Labels
bug Something isn't working

Comments

@harrism
Copy link
Member

harrism commented Nov 16, 2021

Describe the bug
Creating an arena_memory_resource with a value of the constructor parameter maximum_size smaller than its internal minimum superblock size will cause any allocation to fail.

Also, the default values of the parameters initial_size and maximum_size are defined in the detail namespace. Since there is another defaulted parameter (dump_log_on_failure) after these two parameters, this makes it difficult to construct an instance of this MR using the defaults for the sizes but a non-default value for dump_log_on_failure.

Steps/Code to reproduce bug

cuda_mr cuda;
arena_mr mr{&cuda, 1024_B, 32_KiB, true};
mr.allocate(1024_B); // <-- exception

Expected behavior
An exception should be thrown by the constructor if invalid parameters are passed.

@harrism harrism added the bug Something isn't working label Nov 16, 2021
@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@harrism
Copy link
Member Author

harrism commented Jan 10, 2022

@rongou is this fixed in #916 as well?

@rongou
Copy link
Contributor

rongou commented Jan 12, 2022

Added a check and corresponding test.

@rapids-bot rapids-bot bot closed this as completed in #916 Jan 12, 2022
rapids-bot bot pushed a commit that referenced this issue Jan 12, 2022
Currently the arena allocator divides GPU memory into a global arena and per-thread arenas. For smaller allocations, a per-thread arena allocates large chunks of memory (superblocks) from the global arena and divides them up for individual allocations. However, when deallocating from another arena (producer/consumer pattern), or when we run out of memory and return everything to the global arena, the superblock boundaries are broken. Overtime, this could cause the memory to get more and more fragmented.

This PR makes superblocks concrete objects, not just virtual boundaries, and the only units of exchange between the global arena and per-thread arenas. This should make the allocator more resistant to memory fragmentation, especially for long running processes under constant memory pressure.

Other notable changes:
* The allocator now allocates a fixed but configurable amount of memory from CUDA. This introduces less fragmentation comparing to growing the pool size gradually.
* Switched to C++17 `std::shared_mutex`.
* Added a bunch of unit tests.

fixes #919 
fixes #906

Authors:
  - Rong Ou (https://github.com/rongou)

Approvers:
  - Jake Hemstad (https://github.com/jrhemstad)
  - Mark Harris (https://github.com/harrism)

URL: #916
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants