Skip to content

[Perf] Improve SyntaxRewriter visitation performance #2726

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

Merged
merged 8 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[SyntaxRewriter] Introduce SyntaxNodeFactory
Factor out 'Syntax.Info' reusing into 'SyntaxNodeFactory' and
'SyntaxInfoRepository' as the backing storage.
The underlying storage now uses a fixed length 'ManagedBuffer' to avoid
'ContiguousArray' overhead.
  • Loading branch information
rintaro committed Jul 15, 2024
commit b6898d1d653296506824aa684493e7a19f46073f
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,8 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {

DeclSyntax(
"""
/// `Syntax.Info` salvaged from the node being deinitialized in 'visitChildren'.
///
/// Instead of deallocating them and allocating memory for new syntax nodes, store the allocated memory in an array.
/// We can then re-use them to create new syntax nodes.
///
/// The array's size should be a typical nesting depth of a Swift file. That way we can store all allocated syntax
/// nodes when unwinding the visitation stack.
///
/// The actual `info` stored in the `Syntax.Info` objects is garbage. It needs to be set when any of the `Syntax.Info`
/// objects get re-used.
///
/// Note: making the element non-nil causes 'swift::runtime::SwiftTLSContext::get()' traffic somehow.
private var recyclableNodeInfos: ContiguousArray<Syntax.Info?>
/// 'Syntax' object factory recycling 'Syntax.Info' instances.
private let nodeFactory: SyntaxNodeFactory = SyntaxNodeFactory()
"""
)

Expand All @@ -65,8 +54,6 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
public init(viewMode: SyntaxTreeViewMode = .sourceAccurate) {
self.viewMode = viewMode
self.arena = nil
self.recyclableNodeInfos = []
self.recyclableNodeInfos.reserveCapacity(64)
}
"""
)
Expand All @@ -77,8 +64,6 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
public init(viewMode: SyntaxTreeViewMode = .sourceAccurate, arena: SyntaxArena? = nil) {
self.viewMode = viewMode
self.arena = arena
self.recyclableNodeInfos = []
self.recyclableNodeInfos.reserveCapacity(64)
}
"""
)
Expand Down Expand Up @@ -353,15 +338,7 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
}

// Build the Syntax node to rewrite
var childNode: Syntax
if !recyclableNodeInfos.isEmpty {
let recycledInfo: Syntax.Info = recyclableNodeInfos.removeLast()!
recycledInfo.info = .nonRoot(.init(parent: Syntax(node), absoluteInfo: info))
childNode = Syntax(child, info: recycledInfo)
} else {
let absoluteRaw = AbsoluteRawSyntax(raw: child, info: info)
childNode = Syntax(absoluteRaw, parent: syntaxNode)
}
var childNode = nodeFactory.create(parent: syntaxNode, raw: child, absoluteInfo: info)

dispatchVisit(&childNode)
if childNode.raw.id != child.id {
Copy link
Member Author

@rintaro rintaro Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously it compared SyntaxIdentifier which takes the entire tree into account.
Now it compares RawSyntax.ID because we only care the RawSyntax here.

I.e. returning node.detached is NOT considered "rewritten"

Expand Down Expand Up @@ -392,14 +369,8 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
}
}

if recyclableNodeInfos.capacity > recyclableNodeInfos.count,
isKnownUniquelyReferenced(&childNode.info) {
var info: Syntax.Info! = nil
// 'swap' to avoid ref-counting traffic.
swap(&childNode.info, &info)
info.info = nil
recyclableNodeInfos.append(info)
}
// Recycle 'childNode.info'
nodeFactory.dispose(&childNode)
}

if let newLayout {
Expand Down
1 change: 1 addition & 0 deletions Sources/SwiftSyntax/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ add_swift_syntax_library(SwiftSyntax
SyntaxCollection.swift
SyntaxHashable.swift
SyntaxIdentifier.swift
SyntaxNodeFactory.swift
SyntaxNodeStructure.swift
SyntaxProtocol.swift
SyntaxText.swift
Expand Down
85 changes: 85 additions & 0 deletions Sources/SwiftSyntax/SyntaxNodeFactory.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2024 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

/// Reusable 'Syntax.Info' storage.
private struct SyntaxInfoRepository {
private final class _Buffer: ManagedBuffer<Int, Syntax.Info?> {
deinit {
self.withUnsafeMutablePointers { headerPtr, elementPtr in
_ = elementPtr.deinitialize(count: headerPtr.pointee)
}
}
}

/// Fixed capacity which is enough for most use cases.
static var capacity: Int { 64 }

private let buffer: _Buffer

init() {
let buffer = _Buffer.create(minimumCapacity: Self.capacity, makingHeaderWith: { _ in 0 })
self.buffer = buffer as! _Buffer
}

/// Take the 'Syntax.Info' object from the address.
func push(_ info: UnsafeMutablePointer<Syntax.Info?>) {
buffer.withUnsafeMutablePointers { headerPtr, elementPtr in
guard headerPtr.pointee < Self.capacity else {
return
}
assert(info.pointee != nil, "tried to push 'nil' info")
elementPtr.advanced(by: headerPtr.pointee).moveInitialize(from: info, count: 1)
info.initialize(to: nil)
headerPtr.pointee += 1
}
}

/// Vend a 'Swift.Info' object if available.
func pop() -> Syntax.Info? {
return buffer.withUnsafeMutablePointers { headerPtr, elementPtr in
guard headerPtr.pointee > 0 else {
return nil
}
headerPtr.pointee -= 1
return elementPtr.advanced(by: headerPtr.pointee).move()
}
}
}

/// 'Syntax' object factory. This may hold some stocks of recycled 'Syntax.Info'.
struct SyntaxNodeFactory {
private let syntaxInfoRepo: SyntaxInfoRepository = SyntaxInfoRepository()

/// Create a 'Syntax' instance using the supplied info.
///
/// If this factory has a recycled 'Syntax.Info', use one of them. Otherwise, just create a instance by allocating a new one.
@inline(__always)
func create(parent: Syntax, raw: RawSyntax, absoluteInfo: AbsoluteSyntaxInfo) -> Syntax {
if let info = syntaxInfoRepo.pop() {
info.info = .nonRoot(.init(parent: parent, absoluteInfo: absoluteInfo))
return Syntax(raw, info: info)
} else {
return Syntax(raw, parent: parent, absoluteInfo: absoluteInfo)
}
}

/// Dispose a 'Syntax' object.
///
/// 'node.info' is collected for future reuse. 'node' is not usable after calling this.
@inline(__always)
func dispose(_ node: inout Syntax) {
if isKnownUniquelyReferenced(&node.info) {
node.info.unsafelyUnwrapped.info = nil
syntaxInfoRepo.push(&node.info)
}
}
}
39 changes: 5 additions & 34 deletions Sources/SwiftSyntax/generated/SyntaxRewriter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,33 +31,18 @@ open class SyntaxRewriter {
/// intermediate nodes should be allocated.
private let arena: SyntaxArena?

/// `Syntax.Info` salvaged from the node being deinitialized in 'visitChildren'.
///
/// Instead of deallocating them and allocating memory for new syntax nodes, store the allocated memory in an array.
/// We can then re-use them to create new syntax nodes.
///
/// The array's size should be a typical nesting depth of a Swift file. That way we can store all allocated syntax
/// nodes when unwinding the visitation stack.
///
/// The actual `info` stored in the `Syntax.Info` objects is garbage. It needs to be set when any of the `Syntax.Info`
/// objects get re-used.
///
/// Note: making the element non-nil causes 'swift::runtime::SwiftTLSContext::get()' traffic somehow.
private var recyclableNodeInfos: ContiguousArray<Syntax.Info?>
/// 'Syntax' object factory recycling 'Syntax.Info' instances.
private let nodeFactory: SyntaxNodeFactory = SyntaxNodeFactory()

public init(viewMode: SyntaxTreeViewMode = .sourceAccurate) {
self.viewMode = viewMode
self.arena = nil
self.recyclableNodeInfos = []
self.recyclableNodeInfos.reserveCapacity(64)
}

@_spi(RawSyntax)
public init(viewMode: SyntaxTreeViewMode = .sourceAccurate, arena: SyntaxArena? = nil) {
self.viewMode = viewMode
self.arena = arena
self.recyclableNodeInfos = []
self.recyclableNodeInfos.reserveCapacity(64)
}

/// Rewrite `node`, keeping its parent unless `detach` is `true`.
Expand Down Expand Up @@ -3952,15 +3937,7 @@ open class SyntaxRewriter {
}

// Build the Syntax node to rewrite
var childNode: Syntax
if !recyclableNodeInfos.isEmpty {
let recycledInfo: Syntax.Info = recyclableNodeInfos.removeLast()!
recycledInfo.info = .nonRoot(.init(parent: Syntax(node), absoluteInfo: info))
childNode = Syntax(child, info: recycledInfo)
} else {
let absoluteRaw = AbsoluteRawSyntax(raw: child, info: info)
childNode = Syntax(absoluteRaw, parent: syntaxNode)
}
var childNode = nodeFactory.create(parent: syntaxNode, raw: child, absoluteInfo: info)

dispatchVisit(&childNode)
if childNode.raw.id != child.id {
Expand Down Expand Up @@ -3991,14 +3968,8 @@ open class SyntaxRewriter {
}
}

if recyclableNodeInfos.capacity > recyclableNodeInfos.count,
isKnownUniquelyReferenced(&childNode.info) {
var info: Syntax.Info! = nil
// 'swap' to avoid ref-counting traffic.
swap(&childNode.info, &info)
info.info = nil
recyclableNodeInfos.append(info)
}
// Recycle 'childNode.info'
nodeFactory.dispose(&childNode)
}

if let newLayout {
Expand Down