Improve performance of SyntaxVisitor
#2091
Comments
2 tasks
|
Tracked in Apple’s issue tracker as rdar://114330163 |
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
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 apple#2091 rdar://114330163
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
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 apple#2091 rdar://114330163
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
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 apple#2091 rdar://114330163
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
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 apple#2091 rdar://114330163
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
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 apple#2091 rdar://114330163
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
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 apple#2091 rdar://114330163
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
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 apple#2091 rdar://114330163
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
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 apple#2091 rdar://114330163
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
Mar 15, 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 apple#2091 rdar://114330163
ahoppen
added a commit
to ahoppen/swift-syntax
that referenced
this issue
Mar 20, 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 apple#2091 rdar://114330163
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When @StevenWong12 authored #2087, we noticed that the new
SyntaxVisitor-based implementation is 50% slower than the old implementation. It looksSyntaxVisitoris performing quite a few more retain and release calls that might not be necessary. We should investigate if there’s something we can do about that.For newcomers to the project: Note that this is a fairly advanced issue. It will require debugging code at the SIL level and I’m not sure if there’s a road to success.
The text was updated successfully, but these errors were encountered: