-
Notifications
You must be signed in to change notification settings - Fork 440
Remove unused nodes from SyntaxTreeDeserializer.nodeLookupTable #3
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
Conversation
@ahoppen Just to clarify: Even with this changes, |
Yes, that's true. It only now occurred to me that maybe every |
I don't think that is a good idea. A simple question is: why not just recreating the table? private func deserializeByteTree(_ data: Data) throws -> RawSyntax {
var userInfo: [ByteTreeUserInfoKey: Any] = [:]
var newLookupTable: [SyntaxNodeId: RawSyntax] = [:]
userInfo[.rawSyntaxDecodedCallback] = { (node: RawSyntax) -> Void in
newLookupTable[node.id] = node
}
/* ... actual deserialization work .. */
self.lookupTable = newLookupTable
} |
Because if a node gets reused, we won't visit its children that way. And we definitely don't want to do that because it would get us back to O(n) incremental parsing. |
…kupTable This way we don't continue to retain RawSyntax nodes that are no longer needed for incremental transfer.
b40e835
to
0b303c0
Compare
Ah, I understand, thanks. Let me think for a while... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let's merge this AS IS for now. This is definitely an improvement.
We can do further improvement later.
@nkcsgexi Could you review this?
I'm still reviewing this. Please hold on merging. |
I was also worrying about the ever increasing size of |
Included in #11. Closing |
# This is the 1st commit message: fixed testAvailabilityQuery34 and testAvailabilityQueryUnavailability28 # This is the commit message swiftlang#2: Update Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift Co-authored-by: Kim de Vos <[email protected]> # This is the commit message swiftlang#3: added fixedSource in test case # This is the commit message swiftlang#4: minor changes # This is the commit message swiftlang#5: implemented recovery inside the parser # This is the commit message swiftlang#6: runned format.py # This is the commit message swiftlang#7: minor changes # This is the commit message swiftlang#8: minor changes
Make members of a private extension fileprivate.
Adding line breaks inside of #if conditionals is dangerous because line breaks are only valid if the entire condition is wrapped in parens. It's difficult to conditionally add parens only if a break is necessary because it implies backtracking into the printer's output buffer to insert the opening paren. Our options for fixing this problem were: 1. Implement paren wrapping the #if condition if a break fires (very complex). 2. Always paren wrap the #if condition (looks bad in the most common case where there's no breaking). 3. Disable breaking in the pretty print but allow users to explicitly break in the condition using discretionary newlines. I selected option swiftlang#3 and implemented by adding new kind of token known as "printer control" to disable/enable suppressing of break tokens. These printer control tokens can be used in other scenarios, such as suppressing breaks in string interpolation expressions. Fixes SR-11815
ci: Change runner from macOS to Linux
At the moment we keep a reference to all nodes in the
nodeLookupTable
ofSyntaxTreeDeserializer
. The memory usage ofSyntaxTreeDeserializer
always grows over time. After this PR we will remove nodes fromnodeLookupTable
that will never be used for incremental deserialisation.Implements rdar://43516167