[Syntax] Perf: Optimize 'root' and 'ancestorOrSelf' #2842
Conversation
|
@swift-ci Please test |
|
@swift-ci Please test macOS |
| } | ||
|
|
||
| /// "designated" memberwise initializer of `Syntax`. | ||
| @_transparent |
There was a problem hiding this comment.
Do we need @_transparent here? It does make debugging odd because you can no longer step into the function. Isn‘t @inline(__always) enough? Same for the other @_transparent functions. https://github.com/swiftlang/swift/blob/main/docs/TransparentAttr.md for reference.
There was a problem hiding this comment.
Unfortunately normal inlining is too late for eliminating ARC operations for:
func withValue<T>(_ body: (Syntax) -> T) -> T {
info._withUnsafeGuaranteedRef {
body(Syntax(self.raw, info: $0))
}
}In https://godbolt.org/z/j1o6YM4Pz Try changing @_transparent to @inline(__always) on Syntax.init, and see the difference of withValue
There was a problem hiding this comment.
Hmm, that’s unfortunate. Could you add a comment with this explanation to the @_transparent attribute?
Sources/SwiftSyntax/Syntax.swift
Outdated
|
|
||
| /// Temporary non-owning Syntax. | ||
| /// | ||
| /// This can be used for handing Syntax node without ARC traffic. |
There was a problem hiding this comment.
| /// This can be used for handing Syntax node without ARC traffic. | |
| /// This can be used for handling Syntax node without ARC traffic. |
| switch $0.info.unsafelyUnwrapped { | ||
| case .nonRoot(let info): | ||
| return UnownedSyntax(info.parent) | ||
| case .root(_): |
There was a problem hiding this comment.
| case .root(_): | |
| case .root: |
There was a problem hiding this comment.
Looks like we're inconsistent. var nonRootInfo uses case .root(_). I personally prefer (_) style because it's more explicit on ignoring associated values.
| walk = unwrappedParent.parent | ||
| return withUnownedSyntax(self) { | ||
| var node = $0 | ||
| repeat { |
There was a problem hiding this comment.
If this is an infinite loop, would while true be easier to read than repeat { } while true?
rintaro
force-pushed
the
unownedsyntax
branch
3 times, most recently
from
a53bccf to
4fb4c0c
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
| } | ||
|
|
||
| /// "designated" memberwise initializer of `Syntax`. | ||
| @_transparent |
There was a problem hiding this comment.
Hmm, that’s unfortunate. Could you add a comment with this explanation to the @_transparent attribute?
Sources/SwiftSyntax/Syntax.swift
Outdated
| var raw: RawSyntax | ||
| var info: Unmanaged<Syntax.Info> |
There was a problem hiding this comment.
Should these be private (or at least info be private). Seems like the intended access paths are via value and parent.
Sources/SwiftSyntax/Syntax.swift
Outdated
| /// | ||
| /// This guarantees the life time of the `node` during the `body` is executed. | ||
| @inline(__always) | ||
| func withUnownedSyntax<T>(_ node: some SyntaxProtocol, _ body: (UnownedSyntax) -> T) -> T { |
There was a problem hiding this comment.
Would this be better as a member on SyntaxProtocol so that you would do self.withUnownedSyntax instead of withUnownedSyntax(self)?
rintaro
force-pushed
the
unownedsyntax
branch
from
4fb4c0c to
a05c783
Compare
|
swiftlang/swift#76360 |
|
swiftlang/swift#76360 |
2 participants
Accessing
Syntax.parentrequires retain/release traffic. Especially when traversing ancestors to the root, that ARC traffic is not negligible.Introduce
UnownedSyntaxthat provides a way to access the parents temporarily without causing ARC traffics.