Skip to content

Move implementation of each case in SyntaxRewriter.doVisit to separate function #147

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 1 commit into from
Oct 3, 2019

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Oct 3, 2019

In debug builds the compiler lays the stack space for each case
statement out next to each other. This causes a call to doVisit(::) to
use ~50KB of stack space, quickly resulting in a stack overflow. Moving
each implementation into its own function causes the each case to not
use any stack space, circumventing the issue.

Fixes SR-11170

…e function

In debug builds the compiler lays the stack space for each case
statement out next to each other. This causes a call to doVisit(_:_:) to
use ~50KB of stack space, quickly resulting in a stack overflow. Moving
each implementation into its own function causes the each case to not
use any stack space, circumventing the issue. <rdar://55929175>

Fixes SR-11170
@ahoppen ahoppen requested review from akyrtzi and benlangmuir October 3, 2019 17:42
@ahoppen
Copy link
Member Author

ahoppen commented Oct 3, 2019

Based on some rough measurements, this also appears to give us a ~15% performance increase in release and ~35% performance increase in debug builds for the SyntaxVisitor.

@ahoppen
Copy link
Member Author

ahoppen commented Oct 3, 2019

@swift-ci Please test

Copy link
Contributor

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

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

Great improvement, thanks for working through the underlying issue!

Copy link
Contributor

@akyrtzi akyrtzi left a comment

Choose a reason for hiding this comment

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

Whoa, this is a performance improvement in release? Nice!

@ahoppen ahoppen merged commit cd7184a into swiftlang:master Oct 3, 2019
@ahoppen ahoppen deleted the dovisit-extract-case branch November 5, 2019 22:58
adevress pushed a commit to adevress/swift-syntax that referenced this pull request Jan 14, 2024
Use a location specific diagnostic message when the input is invalid.
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.

3 participants