-
Notifications
You must be signed in to change notification settings - Fork 77
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
Attempt to address memory leak described in #198 #214
Draft
dairiki
wants to merge
5
commits into
lovasoa:master
Choose a base branch
from
dairiki:bug.198-memory-leak
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
dairiki
force-pushed
the
bug.198-memory-leak
branch
from
September 24, 2022 20:38
9ddd43c
to
5123577
Compare
dairiki
force-pushed
the
bug.198-memory-leak
branch
2 times, most recently
from
September 25, 2022 21:56
c52cce0
to
766cd57
Compare
Other things that could maybe go into
|
Here we are more careful about which caller's locals we use to resolve forward type references. We want the callers locals at decoration-time — not at decorator-construction time. Consider: ```py frozen_dataclass = marshmallow_dataclass.dataclass(frozen=True) def f(): @custom_dataclass class A: b: "B" @custom_dataclass class B: x: int ``` The locals we want in this case are the one from where the custom_dataclass decorator is called, not from where marshmallow_dataclass.dataclass is called.
When class_schema is called, it doesn't need the caller's whole stack frame. What it really wants is a `localns` to pass to `typing.get_type_hints` to be used to resolve type references. Here we add the ability to pass an explicit `localns` parameter to `class_schema`. We also add the ability to pass an explicit `globalns`, because ... might as well — it might come in useful. (Since we need these only to pass to `get_type_hints`, we might as well match `get_type_hints` API as closely as possible.)
dairiki
force-pushed
the
bug.198-memory-leak
branch
from
January 19, 2023 20:15
766cd57
to
9093446
Compare
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.
Here's a sort of rough and minimal crack at addressing #198.
The main change here is to avoid stack frames winding up in the LRU cache keys by passing the frames in a thread-local global.
I have a nagging feeling that there must be a cleaner way to address this, but this is the best I've come up with so far.
Suggestions welcome.
Bugs Fixed
Fixes #198
Fixes #229