WIP: Avoid recursion in _extract_serialize
#4258
Closed
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
Instead of recursively calling `_extract_serialize`, simply maintain a queue of collections to visit. When starting out, pop the first collection from the queue and process it. As we encounter new collections, add them to the queue to visit later. This way we still have the behavior as if we were recursing, but we lower the overhead by not having to track a deep stack with many things that don't need to be included.
jakirkham
force-pushed
the
avoid_recurse_extract_serialize
branch
from
51a3077
to
41f4a0c
Compare
|
The That said, eliminating recursion here doesn't seem to have much impact. At this point just calling Going to go ahead and close this out. If things change, we can always revisit. |
mrocklin
reviewed
Nov 25, 2020
|
|
||
|
|
||
| def _extract_serialize(x_items, x2, ser, bytestrings, path=()): | ||
| q = deque() |
There was a problem hiding this comment.
If we're using this as only a stack then maybe this should be a list rather than a deque
There was a problem hiding this comment.
Yeah I tried both. This performed better based on the benchmarking I did. That said, it seems that recursion itself is already handled quite efficiently.
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.
This rewrites
_extract_serializeto avoid recursion. Does this by making use of acollections.dequeto track all of the nesteddicts andlists that still need to be visited. A pass through eachdictorlistis completed before passing through any nested ones. Otherwise it behaves mostly like a depth-first search. These are all looped through until no more can be found. Then it returns toextract_serialize.