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

[WIP] Enable pool_memory_resource to deallocate to upstream #571

Closed
wants to merge 21 commits into from

Conversation

harrism
Copy link
Member

@harrism harrism commented Sep 22, 2020

Previously, once the pool memory resource needed to grow its pool from upstream, it could not shrink the pool until the MR was destroyed.

This is a WIP to enable freeing upstream blocks back to the upstream resource, which should enable more flexible composition of resources. Initial benchmarking shows that performance is no worse in RANDOM_ALLOCATIONS_BENCH with this change, and possibly even slightly better.

Next, I realized that the pool_memory_resource maintains a set of allocated blocks that is used for nothing other than being able to print out allocated blocks for debugging. So I moved this behind a define TRACK_ALLOCATIONS which is undefined by default. This is a significant performance benefit (up to 36% faster on RANDOM_ALLOCATIONS_BENCH). Full comparison (second column is % difference in time between new and old -- negative values are improvements).

(rapids) rapids@compose:~/rmm/build/release$ _deps/benchmark-src/tools/compare.py benchmarks ~/rmm/build/cuda-11.0/branch-0.18/release/rand_bench_0.18.json rand_bench_upstream.json 
Comparing /home/mharris/rapids/rmm/build/cuda-11.0/branch-0.18/release/rand_bench_0.18.json to rand_bench_upstream.json
Benchmark                                                  Time             CPU      Time Old      Time New       CPU Old       CPU New
---------------------------------------------------------------------------------------------------------------------------------------
BM_RandomAllocations/pool_mr/1000/1                     -0.2430         -0.2430             1             1             1             1
BM_RandomAllocations/pool_mr/1000/4                     -0.2508         -0.2509             1             1             1             1
BM_RandomAllocations/pool_mr/1000/64                    -0.1270         -0.1273             1             1             1             1
BM_RandomAllocations/pool_mr/1000/256                   -0.1617         -0.1617             1             1             1             1
BM_RandomAllocations/pool_mr/1000/1024                  -0.2040         -0.2040             1             0             1             0
BM_RandomAllocations/pool_mr/1000/4096                  -0.3246         -0.1613             1             0             1             0
BM_RandomAllocations/pool_mr/10000/1                    -0.3662         -0.3662            73            46            73            46
BM_RandomAllocations/pool_mr/10000/4                    -0.3635         -0.3634            78            49            78            49
BM_RandomAllocations/pool_mr/10000/64                   -0.3143         -0.3143            16            11            16            11
BM_RandomAllocations/pool_mr/10000/256                  -0.1800         -0.1799             8             6             8             6
BM_RandomAllocations/pool_mr/10000/1024                 -0.1816         -0.1816             6             5             6             5
BM_RandomAllocations/pool_mr/10000/4096                 -0.3116         -0.1420             7             5             6             5
BM_RandomAllocations/pool_mr/100000/1                   -0.3508         -0.3501         10704          6950         10691          6949
BM_RandomAllocations/pool_mr/100000/4                   -0.3077         -0.3076          3565          2468          3565          2468
BM_RandomAllocations/pool_mr/100000/64                  -0.3164         -0.3164           169           116           169           116
BM_RandomAllocations/pool_mr/100000/256                 -0.1724         -0.1724            76            63            76            63
BM_RandomAllocations/pool_mr/100000/1024                -0.1876         -0.1877            61            50            61            50
BM_RandomAllocations/pool_mr/100000/4096                -0.3150         -0.1755            67            46            55            46

TODO

  • Enable freeing blocks upstream as a result of coalescing.
  • Enable freeing fixed_size_memory_resource blocks upstream

@harrism harrism added the 2 - In Progress Currently a work in progress label Sep 22, 2020
@harrism harrism self-assigned this Sep 22, 2020
@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@harrism harrism added the 5 - Merge After Dependencies Depends on another PR: do not merge out of order label Sep 24, 2020
@harrism
Copy link
Member Author

harrism commented Dec 4, 2020

Testing

@harrism harrism changed the base branch from branch-0.16 to branch-0.18 January 20, 2021 04:43
@harrism harrism added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed 5 - Merge After Dependencies Depends on another PR: do not merge out of order labels Jan 26, 2021
@github-actions github-actions bot added the cpp Pertains to C++ code label Feb 15, 2021
@github-actions
Copy link

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

@harrism
Copy link
Member Author

harrism commented Mar 18, 2021

The optimization in this PR was moved to #702 . Testing of this PR found that freeing upstream leads to poor performance due to thrashing. We decided that instead we should look into making an explicit way to request the pool is compacted and trimmed. This feature is on hold. Closing.

@harrism harrism closed this Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - In Progress Currently a work in progress cpp Pertains to C++ code improvement Improvement / enhancement to an existing function inactive-30d non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants