Skip to content

Do not block initialization of the build server when build server is unresponsive in returning the list of test files #1999

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

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Feb 25, 2025

We were blocking the initialization response on self.buildSystemManager.testFiles, which requires the list of test files to be determined. Make that operation asynchronous so that a slow build server can’t take down all of SourceKit-LSP.

await body(element)
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this need either a wait on each of the tasks or a taskGroup.waitForAll()?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, from the documentation of withTaskGroup. I only learned this now, which is why I had waitForAll in older implementations.

A group waits for all of its child tasks to complete or be canceled before it returns.

@ahoppen
Copy link
Member Author

ahoppen commented Feb 26, 2025

@swift-ci Please test

@ahoppen ahoppen force-pushed the schedule-initial-syntactic-test-population branch from 9b6fb11 to 2dca820 Compare February 26, 2025 17:16
@ahoppen
Copy link
Member Author

ahoppen commented Feb 26, 2025

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Feb 26, 2025

@swift-ci Please test Windows

…element in a collection

This is simpler to reason about than creating a `TaskGroup`.
…unresponsive in returning the list of test files

We were blocking the initialization response on `self.buildSystemManager.testFiles`, which requires the list of test files to be determined. Make that operation asynchronous so that a slow build server can’t take down all of SourceKit-LSP.
@ahoppen ahoppen force-pushed the schedule-initial-syntactic-test-population branch from 2dca820 to bd6fb5d Compare February 27, 2025 00:31
@ahoppen
Copy link
Member Author

ahoppen commented Feb 27, 2025

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Feb 27, 2025

@swift-ci Please test Windows

1 similar comment
@ahoppen
Copy link
Member Author

ahoppen commented Feb 27, 2025

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 0a54c25 into swiftlang:main Feb 28, 2025
3 checks passed
@ahoppen ahoppen deleted the schedule-initial-syntactic-test-population branch February 28, 2025 23:36
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