[allocator] Introduce the abstract class CUDACachingAllocator #87251

zdevito · 2022-10-19T00:27:24Z

Stack from ghstack (oldest at bottom):

This replaces the manual function pointers, making it easier to write
new drop-in allocators.

Note that most allocation goes through the Allocator interface, which
CUDAAllocator inherits from, and this arrangement avoids adding and
additional layer of dispatch along this pathway compared to what existed before.

This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. [ghstack-poisoned]

pytorch-bot · 2022-10-19T00:27:26Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/87251

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures, 6 Pending

As of commit bd1088e:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

…tor" This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. [ghstack-poisoned]

This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. ghstack-source-id: f514834 Pull Request resolved: #87251

…tor" This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. [ghstack-poisoned]

This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. ghstack-source-id: 72d3337 Pull Request resolved: #87251

wconstab

Mostly code-motion, i couldn't spot any typos or issues there.

Couple of nits about using_ flags renaming

perf-wise, I agree you're swapping an equivalent load/deref from a function-pointer to a get()->.

wconstab · 2022-10-19T04:26:45Z

aten/src/ATen/cuda/PeerToPeerAccess.cpp

@@ -42,8 +42,7 @@ bool get_p2p_access(int dev, int dev_to_access) {
 #ifdef USE_ROCM
  bool using_cudaMallocAsync = false;


should the #ifdef USE_ROCM case also be renamed to bool needs_pool_spe... = False?

wconstab · 2022-10-19T04:28:52Z

aten/src/ATen/native/cuda/Copy.cu

@@ -97,8 +97,7 @@ void copy_device_to_device(TensorIterator& iter,
 #ifdef USE_ROCM
      bool using_cudaMallocAsync = false;
 #else
-      bool using_cudaMallocAsync = (CUDACachingAllocator::allocatorBackend() ==
-                                    CUDACachingAllocator::AllocatorBackend::CUDAMALLOCASYNC);
+      bool using_cudaMallocAsync = CUDACachingAllocator::get()->needsPoolSpecificPeerAccess();


rename for consistency?

…tor" This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. [ghstack-poisoned]

zdevito · 2022-10-19T18:03:06Z

@pytorchbot merge

pytorchmergebot · 2022-10-19T18:05:23Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2022-10-19T18:15:31Z

Merge failed

Reason: The following mandatory check(s) failed (Rule superuser):

pull

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

…tor" This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. [ghstack-poisoned]

zdevito · 2022-10-19T18:27:48Z

@pytorchbot merge

pytorchmergebot · 2022-10-19T18:30:03Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2022-10-19T18:45:13Z

Merge failed

Reason: The following mandatory check(s) failed (Rule superuser):

pull

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

…tor" This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. [ghstack-poisoned]

This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. ghstack-source-id: 4eb235e Pull Request resolved: #87251

zdevito · 2022-10-19T18:59:31Z

@pytorchmergebot merge

pytorchmergebot · 2022-10-19T19:01:46Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2022-10-19T19:21:57Z

Merge failed

Reason: The following mandatory check(s) failed (Rule superuser):

pull

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

…tor" This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. [ghstack-poisoned]

This replaces the manual function pointers, making it easier to write new drop-in allocators. Note that most allocation goes through the Allocator interface, which CUDAAllocator inherits from, and this arrangement avoids adding and additional layer of dispatch along this pathway compared to what existed before. ghstack-source-id: bfa71eb Pull Request resolved: #87251

zdevito · 2022-10-19T22:03:54Z

@pytorchbot merge

pytorchmergebot · 2022-10-19T22:05:22Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

github-actions · 2022-10-20T01:18:29Z

Hey @zdevito.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

zdevito mentioned this pull request Oct 19, 2022

[allocator] Move getFreeMutex #87237

Closed

zdevito mentioned this pull request Oct 19, 2022

[allocator] disable tests that don't work for cudaMallocAsyncAllocator #87250

Closed

zdevito requested a review from wconstab October 19, 2022 00:38

wconstab approved these changes Oct 19, 2022

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 19, 2022

pytorchmergebot added the Merged label Oct 20, 2022

pytorchmergebot closed this in 0d2c211 Oct 20, 2022

emcastillo mentioned this pull request Oct 20, 2022

Add Pluggable CUDA allocator backend #86786

Closed

emcastillo mentioned this pull request Nov 24, 2022

[FX] Changes done internally at Facebook pytorch/TensorRT#1456

Merged

7 tasks

facebook-github-bot deleted the gh/zdevito/196/head branch June 8, 2023 19:26

		@@ -42,8 +42,7 @@ bool get_p2p_access(int dev, int dev_to_access) {
		#ifdef USE_ROCM
		bool using_cudaMallocAsync = false;

[allocator] Introduce the abstract class CUDACachingAllocator #87251

[allocator] Introduce the abstract class CUDACachingAllocator #87251

Uh oh!

Conversation

zdevito commented Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/87251

✅ No Failures, 6 Pending

Uh oh!

wconstab left a comment

Choose a reason for hiding this comment

Uh oh!

wconstab Oct 19, 2022

Choose a reason for hiding this comment

Uh oh!

wconstab Oct 19, 2022

Choose a reason for hiding this comment

Uh oh!

zdevito commented Oct 19, 2022

Uh oh!

pytorchmergebot commented Oct 19, 2022

Merge started

Uh oh!

pytorchmergebot commented Oct 19, 2022

Merge failed

Uh oh!

zdevito commented Oct 19, 2022

Uh oh!

pytorchmergebot commented Oct 19, 2022

Merge started

Uh oh!

pytorchmergebot commented Oct 19, 2022

Merge failed

Uh oh!

zdevito commented Oct 19, 2022

Uh oh!

pytorchmergebot commented Oct 19, 2022

Merge started

Uh oh!

pytorchmergebot commented Oct 19, 2022

Merge failed

Uh oh!

zdevito commented Oct 19, 2022

Uh oh!

pytorchmergebot commented Oct 19, 2022

Merge started

Uh oh!

github-actions bot commented Oct 20, 2022

Uh oh!

Uh oh!

zdevito commented Oct 19, 2022 •

edited

Loading

pytorch-bot bot commented Oct 19, 2022 •

edited

Loading