Adoption cache should use WeakValueDictionary. #2495
Merged
Conversation
This file contains 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
We use a cache to preserve sharing-by-reference relationships when "adopting" outside modules into a new top-level functional module context (adoption occurs frequently when using advanced config systems like gin or fiddle, or during interactive development). Previously we were careful not to hold references to Scope objects by having the cache itself be a WeakKeyDictionary, however the scope-keyed entries were just classic dicts, and this was leading to leaks of tracers and arrays ultimately due to long-lived references to submodule objects. By using a WeakValueDictionary to refer to these we avoid these lifetime issues.
|
Fixes #2493 |
Codecov Report
@@ Coverage Diff @@
## main #2495 +/- ##
=======================================
Coverage 78.87% 78.88%
=======================================
Files 48 48
Lines 5118 5120 +2
=======================================
+ Hits 4037 4039 +2
Misses 1081 1081
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We use a cache to preserve sharing-by-reference relationships when "adopting" outside modules into a new top-level functional module context (adoption occurs frequently when using advanced config systems like gin or fiddle, or during interactive development).
Previously we were careful not to hold references to Scope objects by having the cache itself be a WeakKeyDictionary, however the scope-keyed entries were just classic dicts, and this was leading to leaks of tracers and arrays ultimately due to long-lived references to submodule objects from the cache. By using a WeakValueDictionary to refer to these we avoid these lifetime issues.