[AOTInductor] Introduce MaybeOwningAtenTensorHandle for ConstantMap #150275

muchulee8 · 2025-03-31T01:05:46Z

Stack from ghstack (oldest at bottom):

-> [AOTInductor] Introduce MaybeOwningAtenTensorHandle for ConstantMap #150275

Summary:
We used RAIIAtenTensorHandle for ConstantMap, where RAIIAtenTensorHandle
is a unique_ptr, indicating that all memory handling is by the
AOTInductor internally.

In this PR, we introduce ConstantAtenTensorHandle which replaces
RAIIATenTensorHandle. This class holds a raw AtenTensorHandle, and also
owns a RAIIAtenTensorHandle if user decides to delegate memory
management to AOTInductor.

This is a prerequisite for user managed buffer, this PR, however only
introduces this class and make sure it works with existing AOTInductor
and has the default behavior identical as using RAIIAtenTensorHandle.

Test Plan:
Existing tests. No change should be introduced within this PR.

Reviewers:

Subscribers:

Tasks:

Tags:

Summary: We used RAIIAtenTensorHandle for ConstantMap, where RAIIAtenTensorHandle is a unique_ptr, indicating that all memory handling is by the AOTInductor internally. In this PR, we introduce ConstantAtenTensorHandle which replaces RAIIATenTensorHandle. This class holds a raw AtenTensorHandle, and also owns a RAIIAtenTensorHandle if user decides to delegate memory management to AOTInductor. This is a prerequisite for user managed buffer, this PR, however only introduces this class and make sure it works with existing AOTInductor and has the default behavior identical as using RAIIAtenTensorHandle. Test Plan: Existing tests. No change should be introduced within this PR. Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

pytorch-bot · 2025-03-31T01:05:49Z

🔗 Helpful Links

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

📄 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 the bot commands wiki or our office hours

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

✅ No Failures

As of commit d499e16 with merge base 861d2cc ():
💚 Looks good so far! There are no failures yet. 💚

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

desertfire

The plan looks good to me, but we should take this opportunity to do some renaming here. Something like RAIIAtenTensorHandle->OwningAtenTensorHandle and ConstantAtenTensorHandle->MaybeOwningAtenTensorHandle will make the meanings more clear.

…antMap" Summary: We used RAIIAtenTensorHandle for ConstantMap, where RAIIAtenTensorHandle is a unique_ptr, indicating that all memory handling is by the AOTInductor internally. In this PR, we introduce ConstantAtenTensorHandle which replaces RAIIATenTensorHandle. This class holds a raw AtenTensorHandle, and also owns a RAIIAtenTensorHandle if user decides to delegate memory management to AOTInductor. This is a prerequisite for user managed buffer, this PR, however only introduces this class and make sure it works with existing AOTInductor and has the default behavior identical as using RAIIAtenTensorHandle. Test Plan: Existing tests. No change should be introduced within this PR. Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: We used RAIIAtenTensorHandle for ConstantMap, where RAIIAtenTensorHandle is a unique_ptr, indicating that all memory handling is by the AOTInductor internally. In this PR, we introduce MaybeOwningAtenTensorHandle which replaces RAIIATenTensorHandle. This class holds a raw AtenTensorHandle, and also owns a RAIIAtenTensorHandle if user decides to delegate memory management to AOTInductor. This is a prerequisite for user managed buffer, this PR, however only introduces this class and make sure it works with existing AOTInductor and has the default behavior identical as using RAIIAtenTensorHandle. Test Plan: Existing tests. No change should be introduced within this PR. Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 1e558a8 Pull Request resolved: #150275

muchulee8 · 2025-04-03T19:10:46Z

The plan looks good to me, but we should take this opportunity to do some renaming here. Something like RAIIAtenTensorHandle->OwningAtenTensorHandle and ConstantAtenTensorHandle->MaybeOwningAtenTensorHandle will make the meanings more clear.

@desertfire I modified the name of ConstantAtenTensorHandletoMaybeOwningAtenTensorHandle.
I don't think changing RAIIAtenTensorHandle make much sense, as RAII itself already means that the class has ownership and initialize/destructs itself, which users shouldn't be messing with it.

…nstantMap" Summary: We used RAIIAtenTensorHandle for ConstantMap, where RAIIAtenTensorHandle is a unique_ptr, indicating that all memory handling is by the AOTInductor internally. In this PR, we introduce ConstantAtenTensorHandle which replaces RAIIATenTensorHandle. This class holds a raw AtenTensorHandle, and also owns a RAIIAtenTensorHandle if user decides to delegate memory management to AOTInductor. This is a prerequisite for user managed buffer, this PR, however only introduces this class and make sure it works with existing AOTInductor and has the default behavior identical as using RAIIAtenTensorHandle. Test Plan: Existing tests. No change should be introduced within this PR. Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: We used RAIIAtenTensorHandle for ConstantMap, where RAIIAtenTensorHandle is a unique_ptr, indicating that all memory handling is by the AOTInductor internally. In this PR, we introduce MaybeOwningAtenTensorHandle which replaces RAIIATenTensorHandle. This class holds a raw AtenTensorHandle, and also owns a RAIIAtenTensorHandle if user decides to delegate memory management to AOTInductor. This is a prerequisite for user managed buffer, this PR, however only introduces this class and make sure it works with existing AOTInductor and has the default behavior identical as using RAIIAtenTensorHandle. Test Plan: Existing tests. No change should be introduced within this PR. Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 62f7d50 Pull Request resolved: #150275

…nstantMap" Summary: We used RAIIAtenTensorHandle for ConstantMap, where RAIIAtenTensorHandle is a unique_ptr, indicating that all memory handling is by the AOTInductor internally. In this PR, we introduce ConstantAtenTensorHandle which replaces RAIIATenTensorHandle. This class holds a raw AtenTensorHandle, and also owns a RAIIAtenTensorHandle if user decides to delegate memory management to AOTInductor. This is a prerequisite for user managed buffer, this PR, however only introduces this class and make sure it works with existing AOTInductor and has the default behavior identical as using RAIIAtenTensorHandle. Test Plan: Existing tests. No change should be introduced within this PR. Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: We used RAIIAtenTensorHandle for ConstantMap, where RAIIAtenTensorHandle is a unique_ptr, indicating that all memory handling is by the AOTInductor internally. In this PR, we introduce MaybeOwningAtenTensorHandle which replaces RAIIATenTensorHandle. This class holds a raw AtenTensorHandle, and also owns a RAIIAtenTensorHandle if user decides to delegate memory management to AOTInductor. This is a prerequisite for user managed buffer, this PR, however only introduces this class and make sure it works with existing AOTInductor and has the default behavior identical as using RAIIAtenTensorHandle. Test Plan: Existing tests. No change should be introduced within this PR. Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: f0c65e9 Pull Request resolved: #150275

muchulee8 · 2025-04-05T03:21:01Z

@pytorchbot merge

pytorchmergebot · 2025-04-05T03:22:55Z

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

…ytorch#150275) Summary: We used RAIIAtenTensorHandle for ConstantMap, where RAIIAtenTensorHandle is a unique_ptr, indicating that all memory handling is by the AOTInductor internally. In this PR, we introduce ConstantAtenTensorHandle which replaces RAIIATenTensorHandle. This class holds a raw AtenTensorHandle, and also owns a RAIIAtenTensorHandle if user decides to delegate memory management to AOTInductor. This is a prerequisite for user managed buffer, this PR, however only introduces this class and make sure it works with existing AOTInductor and has the default behavior identical as using RAIIAtenTensorHandle. Test Plan: Existing tests. No change should be introduced within this PR. Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: pytorch#150275 Approved by: https://github.com/chenyang78, https://github.com/desertfire

pytorch-bot bot added the ciflow/inductor label Mar 31, 2025

This was referenced Mar 31, 2025

[AOTInductor] Modify test for Memory tracking for memory-related #150269

Closed

[AOTInductor] Add User Managed buffer for AOTI constant buffer. #150276

Closed

muchulee8 requested review from SherlockNoMad, chenyang78 and desertfire March 31, 2025 01:09

muchulee8 added the topic: not user facing topic category label Mar 31, 2025

chenyang78 approved these changes Mar 31, 2025

View reviewed changes

desertfire reviewed Mar 31, 2025

View reviewed changes

muchulee8 changed the title ~~[AOTInductor] Introduce ConstantAtenTensorHandle for ConstantMap~~ [AOTInductor] Introduce MaybeOwningAtenTensorHandle for ConstantMap Apr 3, 2025

muchulee8 requested a review from desertfire April 3, 2025 20:20

desertfire approved these changes Apr 4, 2025

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 5, 2025

pytorchmergebot added the merging label Apr 5, 2025

pytorchmergebot added the Merged label Apr 5, 2025

pytorchmergebot closed this in 60a45eb Apr 5, 2025

pytorchmergebot removed the merging label Apr 5, 2025

github-actions bot deleted the gh/muchulee8/53/head branch May 15, 2025 02:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[AOTInductor] Introduce MaybeOwningAtenTensorHandle for ConstantMap #150275

[AOTInductor] Introduce MaybeOwningAtenTensorHandle for ConstantMap #150275

muchulee8 commented Mar 31, 2025 •

edited

Loading

Uh oh!

pytorch-bot bot commented Mar 31, 2025 •

edited

Loading

Uh oh!

desertfire left a comment

Uh oh!

muchulee8 commented Apr 3, 2025

Uh oh!

muchulee8 commented Apr 5, 2025

Uh oh!

pytorchmergebot commented Apr 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[AOTInductor] Introduce MaybeOwningAtenTensorHandle for ConstantMap #150275

[AOTInductor] Introduce MaybeOwningAtenTensorHandle for ConstantMap #150275

Conversation

muchulee8 commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Mar 31, 2025 • 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/150275

✅ No Failures

Uh oh!

desertfire left a comment

Choose a reason for hiding this comment

Uh oh!

muchulee8 commented Apr 3, 2025

Uh oh!

muchulee8 commented Apr 5, 2025

Uh oh!

pytorchmergebot commented Apr 5, 2025

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

muchulee8 commented Mar 31, 2025 •

edited

Loading

pytorch-bot bot commented Mar 31, 2025 •

edited

Loading