Skip to content

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Mar 20, 2024

  • Explanation: This improves the performance of SyntaxVisitor by minimizing the number of memory allocations we need to perform, resulting in a ~35% performance improvement in EmptyParametersRule of SwiftLint compared to 509.0.0 (and by ~50% compared to 510.0.0).
  • Scope: And client that uses SyntaxVisitor
  • Risk: There’s always an inherent risk when doing some kind of manual memory management but I think we should be safe because we didn’t find any issues in the swift-syntax tests or when running SwiftLint
  • Testing:
    • Correctness: Ran swift-syntax tests and SwiftLint rules
    • Performance: Measured performance of SwiftLint and testEmptyVisitorPerformance and verified on the SIL level that we do a minimal amount of ref-counting
  • Issue: rdar://114330163
  • Reviewer: @bnbarham on Significantly improve performance of SyntaxVisitor #2536

ahoppen added 2 commits March 20, 2024 14:28
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 requested a review from bnbarham as a code owner March 20, 2024 13:33
@ahoppen
Copy link
Member Author

ahoppen commented Mar 20, 2024

@swift-ci Please test

@ahoppen ahoppen merged commit 4638c67 into swiftlang:release/6.0 Mar 20, 2024
@ahoppen ahoppen deleted the ahoppen/6.0/improve-visitor-performance branch March 20, 2024 21:49
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.

2 participants