Fix retain cycles in the Swift runtime #2076
Merged
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
These were ported over from the Java runtime, but they were all deprecated there, and were commented as such here. There is no point having them in the Swift runtime because we don't have legacy code to support.
This fixes some hangovers from the port from Java: * unnecessary type annotations; * failure to use "if let" for nil checks; * comments with Java code in them; * a couple of fields that should have been declared private; * some whitespace issues. No semantic change.
This has been ported over from the Java code, but it was deprecated there.
There's no point having it in the Swift runtime because we don't have the
legacy code to support. Also, it wasn't implemented properly, so it
never worked.
Remove {DFA,IntervalSet}.toString(_:[String?]?)
and the inits in ParserInterpreter and DFASerializer for the same reason.
Switch the unit tests to use the alternate toString(_:Vocabulary).
This was doing nothing for us that we couldn't already get with fatalError, so it was just cluttering things.
This is a port of the equivalent code in the Java runtime. This required a change to the CharStream interface: getText was documented as throwing exceptions, but it wasn't actually declared as such. The UnbufferedCharStream.getText implementation throws exceptions (in order to match the semantics of the Java implementation), so this declaration is now needed, and callsites need to be adjusted appropriately.
This refers back up the tree of RuleContext instances, and meant that the whole tree was leaked. Fix this by making the parent field weak.
This refers back up the parse tree, and meant that the whole tree was leaked. Fix this by making the parent field weak.
This was causing the entire parser to be retained, resulting in a large memory leak. This fix simply changes the reference from ParserATNSimulator to Parser to be unowned. Ditto between Lexer and LexerATNSimulator, except this reference is made weak because LexerATNSimulator.recog is nullable. (That difference is dubious IMHO, but I'm leaving it intact for now.)
…rom. This was causing all the tokens, streams, and lexers to be retained. The primary cycle was because of the backreference at CommonToken.source, and the fact that the token streams buffer the tokens that they create. Fix this by replacing the use of a (TokenSource?, CharStream?) pair with TokenSourceAndStream, which does the same job but references its fields weakly. This means that Token.getTokenSource() and Token.getInputStream() will return valid values as long as you retain the lexer / stream elsewhere, but a Token won't itself retain those things.
token stream that triggered the error.
These are useful for error diagnostics, but if client code wants to throw
the RecognitionException but discard the parser and token stream, then
the fields in RecognitionException need to be cleared.
This adds RecognitionException.{clearRecognizer,clearInputStream} so that
client code can clear those fields if desired. It also makes
RecognitionException.ctx weak, so it will go nil at the same time as
the parser is discarded.
ewanmellor
force-pushed
the
swift-retain-cycles
branch
from
da346c4
to
bb3f7e2
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.
Fix a number of retain cycles in the Swift runtime (see the individual changesets for details). These were causing the lexer, parser, ATN simulators, and tokens all to be retained after a parse.
Add
RecognitionException.{clearRecognizer,clearInputStream}so that client code can clear those fields if desired, thus releasing the parser / token stream if it is not needed for error handling.This depends on PR #2075 (and transitively upon PR #2072) due to merge conflicts.