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

Add a stack to the statistics resource #1563

Merged

Conversation

madsbk
Copy link
Member

@madsbk madsbk commented May 16, 2024

Description

In order to enable more fine-grained statistics, this PR adds a stack to statistics_resource_adaptor

Closes #1332

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@github-actions github-actions bot added Python Related to RMM Python API cpp Pertains to C++ code labels May 16, 2024
@madsbk madsbk force-pushed the statistics_resource_counters_stack branch from cb5cd34 to 5544fd7 Compare May 16, 2024 15:45
@madsbk madsbk changed the title Add a counter stack of the statistics resource Add a counter stack to the statistics resource May 16, 2024
@madsbk madsbk changed the title Add a counter stack to the statistics resource Add a stack to the statistics resource May 16, 2024
@madsbk madsbk force-pushed the statistics_resource_counters_stack branch from 5544fd7 to d6fd147 Compare May 17, 2024 06:28
@madsbk madsbk marked this pull request as ready for review May 17, 2024 10:23
@madsbk madsbk requested review from a team as code owners May 17, 2024 10:23
@madsbk madsbk requested review from wence- and bdice May 17, 2024 10:23
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome, great contribution. Can't wait to see how we can connect it to nvtx and nsys. I have a few comments / questions.

include/rmm/mr/device/statistics_resource_adaptor.hpp Outdated Show resolved Hide resolved
include/rmm/mr/device/statistics_resource_adaptor.hpp Outdated Show resolved Hide resolved
include/rmm/mr/device/statistics_resource_adaptor.hpp Outdated Show resolved Hide resolved
include/rmm/mr/device/statistics_resource_adaptor.hpp Outdated Show resolved Hide resolved
python/rmm/rmm/_lib/memory_resource.pyx Outdated Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
madsbk and others added 3 commits May 23, 2024 08:41
Co-authored-by: Mark Harris <783069+harrism@users.noreply.github.com>
@madsbk madsbk requested a review from harrism May 23, 2024 07:47
include/rmm/mr/device/statistics_resource_adaptor.hpp Outdated Show resolved Hide resolved
python/rmm/rmm/_lib/memory_resource.pyx Outdated Show resolved Hide resolved
python/rmm/rmm/_lib/memory_resource.pyx Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
python/rmm/rmm/tests/test_statistics.py Outdated Show resolved Hide resolved
@madsbk madsbk added breaking Breaking change improvement Improvement / enhancement to an existing function labels May 28, 2024
@madsbk madsbk requested a review from bdice May 29, 2024 09:05
Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nits, a nice demonstration of the flexibility RMM offers.

python/rmm/docs/guide.md Outdated Show resolved Hide resolved
python/rmm/docs/guide.md Outdated Show resolved Hide resolved
python/rmm/docs/guide.md Outdated Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
current_mr = rmm.mr.get_current_device_resource()
yield
finally:
if current_mr is not rmm.mr.get_current_device_resource():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the definition of equality between memory resources is that they are interchangeable for the purposes of allocate/deallocate pairs, which is not sufficient for the usage here.

python/rmm/rmm/statistics.py Show resolved Hide resolved
python/rmm/rmm/statistics.py Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
python/rmm/rmm/statistics.py Outdated Show resolved Hide resolved
madsbk and others added 4 commits May 30, 2024 16:17
Co-authored-by: Lawrence Mitchell <wence@gmx.li>
@madsbk madsbk requested a review from wence- May 30, 2024 14:46
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More doc improvements.

python/rmm/docs/guide.md Outdated Show resolved Hide resolved
python/rmm/docs/guide.md Outdated Show resolved Hide resolved
python/rmm/docs/guide.md Outdated Show resolved Hide resolved
python/rmm/docs/guide.md Outdated Show resolved Hide resolved
python/rmm/docs/guide.md Outdated Show resolved Hide resolved
python/rmm/docs/guide.md Outdated Show resolved Hide resolved
python/rmm/docs/guide.md Outdated Show resolved Hide resolved
madsbk and others added 2 commits June 4, 2024 17:15
Co-authored-by: Mark Harris <783069+harrism@users.noreply.github.com>
@madsbk madsbk requested a review from harrism June 4, 2024 15:20
python/rmm/docs/guide.md Outdated Show resolved Hide resolved
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't wait to see it working in nsys!

madsbk and others added 2 commits June 6, 2024 08:44
Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Mads!

@wence-
Copy link
Contributor

wence- commented Jun 6, 2024

/merge

@rapids-bot rapids-bot bot merged commit 6902af9 into rapidsai:branch-24.08 Jun 6, 2024
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change cpp Pertains to C++ code improvement Improvement / enhancement to an existing function Python Related to RMM Python API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Standardize shared_mutex usage
4 participants