Peak memory use optimizations. #199
Merged
+40
−16
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
Reduce peak memory usage of swift-format by discarding collections once they are no longer needed. - Remove `Token` arrays from `beforeMap` and `afterMap` while processing the associated token. - Empty `preVisitedExprs` after visitation finishes for the root expression of the tree. By the end of a file, these maps can accumulate a meaningful number of tokens such that they significantly impact the peak memory usage. This admittedly replaces a O(1) lookup operation with a O(N) lookup-and-remove operation, by replacing `subscript` with `removeValue(forKey:)`. In my testing, I found that this had no impact on the runtime. My hypothesis is that removing from these dictionaries keeps them small enough that O(N) runtime isn't a problem. This set previously contained `ExprSyntax` objects and was maintained for the entire file. Once the visitor has finished with all children of the node that initiated inserting the contextual breaks, it will never visit any of the children again. This set can be emptied at that point to keep it from growing until the visitor finishes with the whole file. The runtime is unchanged, but this reduces peak memory usage especially for large files. - Large file (30k LOC): 108.3 MB peak to 72.1 MB peak - Medium file (10k LOC) : 35.9 MB peak to 24.4 MB peak - swift-protobuf library (many small files): 77.6 MB peak to 75.6 MB peak
dylansturg
force-pushed
the
less_memory_makes_me_happy
branch
from
7ecb041 to
4c8587f
Compare
aaditya-chandrasekhar
pushed a commit
to val-verde/swift-format
that referenced
this pull request
May 20, 2021
aaditya-chandrasekhar
pushed a commit
to val-verde/swift-format
that referenced
this pull request
May 20, 2021
aaditya-chandrasekhar
pushed a commit
to val-verde/swift-format
that referenced
this pull request
May 20, 2021
Verifies that generated `all.css` contains the `:root` string on CI. It doesn't do any actual CSS parsing to verify its full validity, only does a simple `grep` check, but I hope this will be enough to prevent issues like swiftlang#198 in the future. * Test that `all.css` contains a valid CSS selector * Update `CHANGELOG.md` * Install graphviz to avoid warnings on macOS * Add EndToEndTests subclassing XCTest * Apply suggestions from code review Co-authored-by: Mattt <[email protected]> * Remove stray comment leftover from review * Set macOS 10.13 as minimum platform target * Refactor EndToEndTests Create helper methods on Process and Bundle * Test whether generated HTML output contains subdirectories * Add test for CommonMark output * Add tests for coverage subcommand * Add tests for diagram subcommand * Update Changelog entry for swiftlang#199 * Work around unavailability of hasDirectoryPath * Update Changelog.md Co-authored-by: Max Desiatov <[email protected]> * Update Changelog entry for swiftlang#199 Co-authored-by: Mattt <[email protected]>
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.
Reduce peak memory usage of swift-format by discarding collections once they are no longer needed.
Tokenarrays frombeforeMapandafterMapwhile processing the associated token.preVisitedExprsafter visitation finishes for the root expression of the tree.beforeMapandafterMapBy the end of a file, these maps can accumulate a meaningful number of tokens such that they significantly impact the peak memory usage.
This admittedly replaces a O(1) lookup operation with a O(N) lookup-and-remove operation, by replacing
subscriptwithremoveValue(forKey:). In my testing, I found that this had no impact on the runtime. My hypothesis is that removing from these dictionaries keeps them small enough that O(N) runtime isn't a problem.preVisitedExprsThis set previously contained
ExprSyntaxobjects and was maintained for the entire file. Once the visitor has finished with all children of the node that initiated inserting the contextual breaks, it will never visit any of the children again. This set can be emptied at that point to keep it from growing until the visitor finishes with the whole file.Results
The runtime is unchanged, but this reduces peak memory usage especially for large files.