Repository navigation
Metal backend: free only the destroyed delegate's memory #24782
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
base: main
Are you sure you want to change the base?
Changes from all commits
0abc566
9471b22
0a4a1f5
4ad8cea
a3acce5
89dbe34
67d8d5d
090bf1a
d108d69
110723a
ba664e1
1d6b4c4
ddfadcd
903f9f7
f76c5e4
f31c7ec
2240830
34bf7ee
67e0c98
64917ca
093b315
6815df1
36a6283
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -363,6 +363,18 @@ class ET_EXPERIMENTAL MetalBackend final | |
| handle->so_handle = so_handle; | ||
| handle->so_path = so_path.string(); | ||
|
|
||
| // The shims record what this delegate creates while it loads, so that | ||
| // destroy() frees only this delegate's memory. If loading fails, what it | ||
| // made so far is freed here, since destroy() will not be called. | ||
| metal_begin_delegate(handle); | ||
| MetalDelegateScope delegate_scope(handle, /*loading=*/true); | ||
| bool initialized = false; | ||
| executorch::backends::aoti::ScopeGuard release_on_failure([&]() noexcept { | ||
| if (!initialized) { | ||
| metal_release_delegate(handle); | ||
| } | ||
| }); | ||
|
|
||
| // Load function pointers specific to this handle's shared library | ||
| ET_CHECK_OK_OR_RETURN_ERROR( | ||
| load_function_pointers_into_handle(so_handle, handle)); | ||
|
|
@@ -393,6 +405,7 @@ class ET_EXPERIMENTAL MetalBackend final | |
| } | ||
|
|
||
| ET_LOG(Info, "MetalBackend::init - Initialization completed successfully"); | ||
| initialized = true; | ||
|
|
||
| #ifdef EXECUTORCH_METAL_COLLECT_STATS | ||
| // Accumulate init timing statistics | ||
|
|
@@ -447,6 +460,9 @@ class ET_EXPERIMENTAL MetalBackend final | |
| }); | ||
|
|
||
| AOTIDelegateHandle* handle = (AOTIDelegateHandle*)handle_; | ||
| // Handles created by this run, e.g. folded constants that outlive it, | ||
| // belong to this delegate. | ||
| MetalDelegateScope delegate_scope(handle, /*loading=*/false); | ||
|
|
||
| ET_LOG(Debug, "MetalBackend Handle generated"); | ||
|
|
||
|
|
@@ -679,9 +695,14 @@ class ET_EXPERIMENTAL MetalBackend final | |
| } | ||
| } | ||
|
|
||
| // Free this delegate's handles and constants. The shim state, including | ||
| // the buffer pool its recycled buffers went to, is shared by every | ||
| // delegate in the process, so it is only reset once none is left. | ||
| if (metal_release_delegate(handle)) { | ||
| cleanup_memory(); | ||
| executorch::backends::aoti::cleanup_tensor_metadata(); | ||
|
Comment on lines
+701
to
+703
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, the module tests run one program per process. I added |
||
| } | ||
| delete handle; | ||
| cleanup_memory(); | ||
| executorch::backends::aoti::cleanup_tensor_metadata(); | ||
| ET_LOG(Debug, "MetalBackend handle %p destroy", handle_); | ||
| } | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked for a way to make
initfail after it allocates. With a valid program, nothing ininitfails once it has allocated; a bad weights blob makes AOTI read out of bounds instead of returning an error. So this would need a fault-injection hook ininit. The release the guard runs is the one the shim tests cover. Happy to add a hook if you'd like one.