Reduce usage of currentToken
#1909
Conversation
|
@swift-ci Please test |
| /// In assert builds, this traps if the loop has not made any parser progress in between two iterations, | ||
| /// ie. it checks if the parser's `currentToken` has changed in between two calls to `evaluate`. | ||
| /// In non-assert builds, `evaluate()` returns `false` if the loop has not made progress, thus aborting the loop. | ||
| mutating func evaluate(_ currentToken: Lexer.Lexeme) -> Bool { | ||
| mutating func evaluate(_ parser: some TokenConsumer) -> Bool { |
There was a problem hiding this comment.
Parser and Parser.Lookahead are both structs right? Do we really want to copy that in here on every evaluate?
There was a problem hiding this comment.
Hmm, interesting. I actually just played around with it again and was able to improve performance by 0.5% with a @inline(__always) entry function. Not entirely sure why, but 🏎️
There was a problem hiding this comment.
My preference here would be a Parser function eg. hasProgressed that takes a LoopProgressCondition instead. The latter probably isn't the correct name any more either though 😅
There was a problem hiding this comment.
Done. I kept the name as LoopProgressCondition because I didn’t have a better idea and it does describe what the condition is being used for.
ahoppen
force-pushed
the
ahoppen/parser-instead-of-currenttoken
branch
2 times, most recently
from
3d0d3d1 to
4418dc6
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
ahoppen
force-pushed
the
ahoppen/parser-instead-of-currenttoken
branch
from
4418dc6 to
0028719
Compare
|
@swift-ci Please test |
ahoppen
force-pushed
the
ahoppen/parser-instead-of-currenttoken
branch
from
0028719 to
f9982f0
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
ahoppen
force-pushed
the
ahoppen/parser-instead-of-currenttoken
branch
from
f9982f0 to
526ce24
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
ahoppen
force-pushed
the
ahoppen/parser-instead-of-currenttoken
branch
from
526ce24 to
e85b5d2
Compare
… `Lexeme` This slightly simpifies the call sides.
This improves parsing performance by 1.5%.
ahoppen
force-pushed
the
ahoppen/parser-instead-of-currenttoken
branch
from
e85b5d2 to
0554638
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
The motivation for these changes is to reduce the usage of
Parser.currentTokenbecause in general theatprimitives should be preferred to checking thecurrentTokenitself. But independent of these motivations, I think the changes simplify the codebase on their own.LoopProgress.evaluateto take aTokenConsumer(i.e.ParserorLookahead) instead ofcurrentToken. This shortens the call sides slightlyatStartOfLinetoParserto shortenself.currentToken.isAtStartOfLinetoself.atStartOfLineI measured performance and there’s no significant performance regression associated with this change (based on parsing MovieSwiftUI).