Qualcomm: stop an expired bundle from making the backend cache stale - #21944
Open
psiddh wants to merge 1 commit into
Open
Qualcomm: stop an expired bundle from making the backend cache stale#21944psiddh wants to merge 1 commit into
psiddh wants to merge 1 commit into
Conversation
QnnBackendUnifiedRegistry keeps backend bundles in an unordered_map<backend_type, weak_ptr<QnnBackendBundle>>. When the last QnnManager for a type goes away the bundle expires, leaving a dead weak_ptr under that key. The insert on the create path used emplace, which does not replace an existing key, so the dead entry stayed put: every subsequent lookup failed to lock, built a new backend, and again failed to record it. From the first expiry onward the cache never hit, so a live bundle could sit unused while a second QnnBackend and QnnDevice were created for the same backend type. Reproduced with one compile spec: create a manager and destroy it, keep a second alive, then ask for a third. Before this change the third builds its own backend; after it reuses the second's bundle. CleanupExpired() is not usable from GetOrCreateBackendBundle -- it takes mutex_, which is already held there -- hence insert_or_assign. Authored with Claude Code.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21944
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 7d4c42f with merge base b45e85d ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
psiddh
requested review from
shewu-quic
and removed request for
abhinaykukkadapu and
Copilot
August 19, 2026 15:46
psiddh
added
the
module: qnn
This PR needs a
|
psiddh
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
QnnBackendUnifiedRegistry caches backend bundles as weak_ptr keyed on backend type,
and CleanupExpired() only runs in the registry destructor. When a bundle's last
owner goes away, the dead weak_ptr stays in the map. The next request finds the
entry, fails to lock it, builds a fresh bundle, and then calls emplace() — which is
a no-op because the key already exists. The corpse is never replaced, so every
subsequent lowering rebuilds the backend and device instead of sharing them.
insert_or_assign replaces the expired entry. CleanupExpired() can't be used here
because it takes mutex_, which is already held.
Single-lowering processes never reach this, which is why it has been invisible. It
shows up once a process does two or more lowerings: with the fix the second
lowering reuses the cached bundle instead of creating a second one, which also
avoids repeated device configuration.
Test: test_backend_bundle_cache_survives_an_expired_entry asserts two creations
plus at least one reuse, and fails on unmodified main.
cc @cbilgin