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.

Sign up for GitHub

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

[Perf] Improve SyntaxRewriter visitation performance #2726

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6be8e8d
[SyntaxRewriter] Optimize SyntaxRewriter visitation
rintaro Jul 12, 2024
b6898d1
[SyntaxRewriter] Introduce SyntaxNodeFactory
rintaro Jul 13, 2024
e664473
[SyntaxRewriter] Improve new layout node creation
rintaro Jul 14, 2024
e2d4423
[SyntaxRewriter] Make 'visitChildren()' a non-generic mehod
rintaro Jul 15, 2024
b67e899
[SyntaxRewriter] Iterate RawSyntaxChildren using pattern matching
rintaro Jul 15, 2024
7298fe8
[SyntaxVisitor] Adopt SyntaxNodeFactory
rintaro Jul 15, 2024
ef8bb05
[SyntaxRewriter] Return the node as-is
rintaro Jul 18, 2024
ec96b4d
[CMake] Split 'touch' workaround into two commands
rintaro Jul 18, 2024
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] Iterate RawSyntaxChildren using pattern matching 
Simpify the iteration code.
Also, stop counting 'childIndex' as it can be retrieved from the
absolute info.
  • Loading branch information
@rintaro
rintaro committed Jul 15, 2024
commit b67e89984b149acf1612c2d2c8d97921eea597a2
12 changes: 2 additions & 10 deletions CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxRewriterFile.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,15 +318,7 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
// with 'Syntax'
var rewrittens: ContiguousArray<RetainedSyntaxArena> = []

// Incrementing i manually is faster than using .enumerated()
var childIndex = 0
for (raw, info) in RawSyntaxChildren(node) {
defer { childIndex += 1 }

guard let child = raw, viewMode.shouldTraverse(node: child) else {
// Node does not exist or should not be visited.
continue
}
for case let (child?, info) in RawSyntaxChildren(node) where viewMode.shouldTraverse(node: child) {

// Build the Syntax node to rewrite
var childNode = nodeFactory.create(parent: node, raw: child, absoluteInfo: info)
Expand All @@ -343,7 +335,7 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
}

// Update the rewritten child.
newLayout[childIndex] = childNode.raw
newLayout[Int(info.indexInParent)] = childNode.raw
// Retain the syntax arena of the new node until it's wrapped with Syntax node.
rewrittens.append(childNode.raw.arenaReference.retained)
}
Expand Down
14 changes: 2 additions & 12 deletions Sources/SwiftSyntax/generated/SyntaxRewriter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3915,17 +3915,7 @@ open class SyntaxRewriter {
// with 'Syntax'
var rewrittens: ContiguousArray<RetainedSyntaxArena> = []
Copy link
Member

Choose a reason for hiding this comment

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

What are the performance characteristics of ContiguousArray vs Array here? I would expect Array to be faster because it doesn’t need to move all elements if it runs out of space. And I don’t think we need the contiguous property here anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, I haven't tried Array here. interesting.

Copy link
Member Author

Choose a reason for hiding this comment

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

Array always uses contiguous storage unless it's backed by NSArray.
Although there's no particular reason to use ContiguousArray, its implementation is simpler than Array. So let's keep this as-is


// Incrementing i manually is faster than using .enumerated()
var childIndex = 0
for (raw, info) in RawSyntaxChildren(node) {
defer {
childIndex += 1
}

guard let child = raw, viewMode.shouldTraverse(node: child) else {
// Node does not exist or should not be visited.
continue
}
for case let (child?, info) in RawSyntaxChildren(node) where viewMode.shouldTraverse(node: child) {

// Build the Syntax node to rewrite
var childNode = nodeFactory.create(parent: node, raw: child, absoluteInfo: info)
Expand All @@ -3942,7 +3932,7 @@ open class SyntaxRewriter {
}

// Update the rewritten child.
newLayout[childIndex] = childNode.raw
newLayout[Int(info.indexInParent)] = childNode.raw
// Retain the syntax arena of the new node until it's wrapped with Syntax node.
rewrittens.append(childNode.raw.arenaReference.retained)
}
Expand Down