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.

Sign up for GitHub

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

Jump to bottom

[FEA] Include attempted allocation size in rmm::out_of_memory #1134

Closed
abellina opened this issue Oct 21, 2022 · 0 comments · Fixed by #1827
Closed

[FEA] Include attempted allocation size in rmm::out_of_memory #1134

abellina opened this issue Oct 21, 2022 · 0 comments · Fixed by #1827
Assignees
@pmattione-nvidia
Labels
? - Needs Triage Need team to review and classify feature request New feature or request

Comments

@abellina
Copy link
Contributor

abellina commented Oct 21, 2022 •
edited
Loading

It would be useful if rmm::out_of_memory exceptions could include the attempted allocation size that failed.

"Maximum pool size exceeded (failed to allocate X MiB)"

I am thinking it could just be in the msg, but perhaps it could be added as metadata in the out_of_memory class? I am not entirely sure.

ASYNC relevant code:

rmm/include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp

Line 212 in 5e5d82b

RMM_EXPECTS(size <= this->underlying().get_maximum_allocation_size(),

ARENA relevant code:

rmm/include/rmm/mr/device/arena_memory_resource.hpp

Line 159 in 5e5d82b

RMM_FAIL("Maximum pool size exceeded", rmm::out_of_memory);

The change should also include other allocators.
@abellina abellina added ? - Needs Triage Need team to review and classify feature request New feature or request labels Oct 21, 2022
@abellina abellina mentioned this issue Oct 21, 2022
Closed
@bdice bdice mentioned this issue Jan 25, 2025
Closed
@pmattione-nvidia pmattione-nvidia mentioned this issue Feb 13, 2025
Merged
3 tasks
rapids-bot bot pushed a commit that referenced this issue Feb 26, 2025
@pmattione-nvidia
Add OOM fail reason, attempted allocation size to exception messages (#…
d1a5f1b 
…1827)

This PR addresses the following issues:
1. Closes [Issue 1791](#1791): It forwards on the details for why we got an OOM exception in `try_to_expand()`.  It also does a more thorough job of forwarding on failure details in other locations. A test has been added to test this case explicitly. 
2. Closes [Issue 1134](#1134): It adds the size of attempted allocations to the OOM and bad_alloc exceptions. 

Notes:
1. These code paths are already tested by the numerous `EXPECT_THROW()` macros already in the tests, and one more test was explicitly added. 
2. The modified loop in `try_to_expand()` in `pool_memory_resource.hpp` attempts to allocate for the case where `try_size` is less than `min_size`, whereas before it just immediately errored. If this isn't the behavior we want I can change it. Previously it was just erroring that we didn't have enough memory, which isn't right either.

Authors:
  - Paul Mattione (https://github.com/pmattione-nvidia)
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - Alessandro Bellina (https://github.com/abellina)
  - Bradley Dice (https://github.com/bdice)

URL: #1827
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pmattione-nvidia pmattione-nvidia

Labels
? - Needs Triage Need team to review and classify feature request New feature or request
Projects
RMM Project Board
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add OOM fail reason, attempted allocation size to exception messages pmattione-nvidia/rmm
2 participants
@abellina @pmattione-nvidia