Stack overflow v2 #44
Open
Conversation
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
…think), and has immeasurable performance impact
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.
The problem from #43 persists with a different test case. The fix from ce52cf6 does not prevent cases where the stack grows under a certain limit in each scope, but goes over the limit across all scopes. This proposed change carries the same stackSize variable into each scope, so there's no way for any combination of large(ish) objects and arrays to overflow the stack. This change shows the same performance as the previous fix from ce52cf6 (which was indeed a vast improvement over the simple approach I took in fc77a5c).
Two new test cases are included. Maybe more than necessary. I also don't really like the huge diffs these test cases generate, I might look into generating the
.txtcontents at runtime.