Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Nov 5, 2021

Othwerise we were performing the syntactic parsing on a background queue that had a reduced stack size which could result in stack overflows.

rdar://84474387

@ahoppen ahoppen requested a review from bnbarham November 5, 2021 09:21
@ahoppen
Copy link
Member Author

ahoppen commented Nov 5, 2021

@swift-ci Please smoke 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.

LGTM, nice work on finding that one.

@akyrtzi
Copy link
Contributor

akyrtzi commented Nov 5, 2021

Could we only wrap the parsing part in a deep-stack WorkQueue? Kicking off a new thread for every request is a bit heavyweight since we don't implement it as a pool (it was intended for heavyweight operations like typechecking, so optimizing out the overhead of starting a new thread for that didn't seem very important).

@ahoppen ahoppen changed the title [SourceKit] Use deep stack when serving request in the XPC service [SourceKit] Use deep stack when parsing in the XPC service Nov 8, 2021
@ahoppen
Copy link
Member Author

ahoppen commented Nov 8, 2021

@swift-ci Please smoke test

Othwerise we were performing the syntactic parsing on a background queue that had a reduced stack size which could result in stack overflows.

rdar://84474387
@ahoppen
Copy link
Member Author

ahoppen commented Nov 9, 2021

Changed it to only switch to a thread with a deep stack when actually performing the parsing and added a test case because we are no longer hitting an assertion failure now that #40065 is merged.

@ahoppen
Copy link
Member Author

ahoppen commented Nov 9, 2021

@swift-ci Please smoke test

@ahoppen ahoppen merged commit ac56bfb into swiftlang:main Nov 15, 2021
@ahoppen ahoppen deleted the pr/deep-stack branch November 15, 2021 08:17
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