Skip to content

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Mar 8, 2024

There are two core ideas here:

  • Before this change, we were allocating Syntax.Info for every visited node. We need a heap-allocated object here because Syntax can access its parent. In most cases, however, the syntax node was not stored by the visitor and we would just destroy the Syntax.Info after finishing the visit call. Instead, check if Syntax.Info is uniquely referenced, which means that the the node was not stored. In that case, don’t deallocate the memory but place it into a reuse pool. I have seen reductions in malloc calls from ~50,000 to ~100 (500x) in testEmptyVisitorPerformance.
  • Now retain and release calls made up a significant portion of the visitor’s performance. Add some annotations to eliminate reference counting. I added comments for the remaining retain calls.

I measured that this improves the performance of the EmptyParametersRule in SwiftLint by ~35% compared to 509.0.0 (and by ~50% compared to 510.0.0).

Fixes #2091
rdar://114330163

@ahoppen ahoppen requested a review from bnbarham as a code owner March 8, 2024 18:28
@ahoppen ahoppen requested a review from rintaro March 8, 2024 18:37
@ahoppen
Copy link
Member Author

ahoppen commented Mar 8, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Mar 15, 2024

Added a commit that stores which slots in recyclableNodeInfos are free. I didn’t take ultra exact measurements but it seems to give us another 2-4% performance boost in SwiftLint times.

ahoppen added 2 commits March 14, 2024 17:54
There are two core ideas here:
- Before this change, we were allocating `Syntax.Info` for every visited node. We need a heap-allocated object here because `Syntax` can access its parent. In most cases, however, the syntax node was not stored by the visitor and we would just destroy the `Syntax.Info` after finishing the `visit` call. Instead, check if `Syntax.Info` is uniquely referenced, which means that the the node was not stored. In that case, don’t deallocate the memory but place it into a reuse pool. I have seen reductions in `malloc` calls from ~50,000 to ~100 (500x) in `testEmptyVisitorPerformance`.
- Now retain and release calls made up a significant portion of the visitor’s performance. Add some annotations to eliminate reference counting. I added comments for the remaining retain calls.

I measured that this improves the performance of the `EmptyParametersRule` in SwiftLint by ~35% compared to `509.0.0` (and by ~50% compared to `510.0.0`).

Fixes swiftlang#2091
rdar://114330163
This gets us another ~3% performance improvement in SwiftLint listing time.
@ahoppen ahoppen force-pushed the ahoppen/improve-visitor-performance branch from ece8775 to 31575ca Compare March 15, 2024 00:54
@ahoppen
Copy link
Member Author

ahoppen commented Mar 15, 2024

@swift-ci Please test

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

Nice!

@ahoppen
Copy link
Member Author

ahoppen commented Mar 15, 2024

@swift-ci Please test

@ahoppen ahoppen merged commit 0a3851b into swiftlang:main Mar 20, 2024
@ahoppen ahoppen deleted the ahoppen/improve-visitor-performance branch March 20, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve performance of SyntaxVisitor

2 participants