[Syntax] Perf: Optimize 'root' and 'ancestorOrSelf' #2842
Conversation
|
@swift-ci Please test |
|
@swift-ci Please test macOS |
| @@ -129,6 +132,7 @@ public struct Syntax: SyntaxProtocol, SyntaxHashable { | |||
| } | |||
|
|
|||
| /// "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
| @@ -380,6 +384,57 @@ extension Syntax { | |||
| } | |||
| } | |||
|
|
|||
| /// 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 |
| @@ -129,6 +132,7 @@ public struct Syntax: SyntaxProtocol, SyntaxHashable { | |||
| } | |||
|
|
|||
| /// "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 |
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.