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

[FEA] Extend tracking_resource_adaptor to track current bytes allocated, peak bytes allocated, and total # allocations #622

Closed
hyperbolic2346 opened this issue Nov 2, 2020 · 8 comments · Fixed by #626
Labels
feature request New feature or request inactive-30d Python Related to RMM Python API

Comments

@hyperbolic2346
Copy link
Contributor

hyperbolic2346 commented Nov 2, 2020

@mdemoret-nv suggests that tracking_resource_adaptor should also keep track of total bytes allocated, current outstanding bytes, peak bytes allocated, and total number of allocations.

For the cuml team, this would be really helpful (@mdemoret-nv suggested he can make the changes). "Having the ability to track peak allocations is very useful for us since this ultimately decides the max size on datasets that can be loaded. And being able to query/reset this information from python would be allow tracking this information by test."

Originally posted by @mdemoret-nv in #596 (comment)

@harrism harrism changed the title Additions requested for tracking adaptor [FEA] Extend tracking_resource_adaptor to track current bytes allocated, peak bytes allocated, and total # allocations Nov 2, 2020
@harrism harrism added this to Needs prioritizing in Feature Planning via automation Nov 2, 2020
@harrism harrism added feature request New feature or request Python Related to RMM Python API labels Nov 2, 2020
@harrism
Copy link
Member

harrism commented Nov 2, 2020

I would suggest also tracking the peak number of allocations as well. So a high water mark for peak bytes and peak #allocations.

@github-actions
Copy link

This issue has been marked rotten due to no recent activity in the past 90d. 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.

@github-actions
Copy link

This issue has been marked stale due to no recent activity in the past 30d. 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 marked rotten if there is no activity in the next 60d.

@harrism
Copy link
Member

harrism commented Feb 16, 2021

Still relevant and being worked by #626

@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.

@hyperbolic2346
Copy link
Contributor Author

This is certainly still relevant.

@harrism
Copy link
Member

harrism commented Apr 6, 2021

I think statistics tracking should be in a separate adaptor from allocation tracking for leak detection purposes.

@github-actions
Copy link

github-actions bot commented May 6, 2021

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.

@rapids-bot rapids-bot bot closed this as completed in #626 Jun 8, 2021
Feature Planning automation moved this from Needs prioritizing to Closed Jun 8, 2021
rapids-bot bot pushed a commit that referenced this issue Jun 8, 2021
…urce_adaptor` and `statistics_resource_adaptor` (#626)

Closes #622 and Closes #623 

This PR updates the C++ `tracking_resource_adaptor` with stack trace information and also adds a new MR, `statistics_resource_adaptor`. Summary of all changes:

- `tracking_resource_adaptor` changes:
   - Added Cython wrapper `rmm.mr.TrackingResourceAdaptor` which wraps all available methods
   - Updated `tracking_resource_adaptor` to correctly log stack trace information with `capture_stacks=True`
- Added `statistics_resource_adaptor` memory resource:
   - This MR will keep track of the current, peak and total allocated bytes and number of allocations
   - Added Cython wrapper `rmm.mr.StatisticsResourceAdaptor` which wraps all available methods

These two MR can be used separately and together to track memory allocations, check for memory leaks, and identify incorrect deallocations. While both MRs can track the current number of allocations, they have different areas of focus. 

The `tracking_resource_adaptor` is designed more towards identifying and fixing memory leaks, and will log stack trace information for every memory allocation. This MR will have significant performance impacts since it logs a large amount of information for every allocation. 

The `statistics_resource_adaptor` is a lightweight MR that adds simple counters to track the allocated bytes and allocation count. This MR will have significantly less of a performance impact but cannot identify the cause of memory leaks, only that they exist. This MR is also great at tracking peak memory usage and can be helpful in identifying areas that require large amounts of memory or helping developers measure memory usage reductions during optimization.

Authors:
  - Michael Demoret (https://github.com/mdemoret-nv)

Approvers:
  - Keith Kraus (https://github.com/kkraus14)
  - Rong Ou (https://github.com/rongou)
  - Mark Harris (https://github.com/harrism)
  - Jake Hemstad (https://github.com/jrhemstad)
  - GALI PREM SAGAR (https://github.com/galipremsagar)

URL: #626
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request inactive-30d Python Related to RMM Python API
Projects
No open projects
2 participants