Skip to content

[Explicit Module Builds] Use the new batch scanning mode for versioned PCM clang modele re-scan #236

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
Sep 8, 2020

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Sep 2, 2020

Another attempt at landing #225.

----------- Original Description -----------
swiftlang/swift#33569 introduced a batch dependency scanning mode to save up on the overhead of clang instance creation.
This patch switches swift-driver over to using the new batch mode instead of generating individual -scan-clang-dependencies jobs for each re-scan.

@artemcm
Copy link
Contributor Author

artemcm commented Sep 2, 2020

I haven't been able to reproduce the swift CI failure locally yet so debugging that issue is still WIP.

@artemcm
Copy link
Contributor Author

artemcm commented Sep 3, 2020

Does not seem that the CI failure is a code-interaction issue, it just seems to intermittently happen.

@artemcm
Copy link
Contributor Author

artemcm commented Sep 4, 2020

Ah. The bug seems to be test-specific. Across different tests, instances of Driver share a temporary directory that we get from TSCBasic, because the path to the temporary directory is cached as a global variable. Tests also share the main module name (main). The input to the batch scanning is computed using the temporary directory and the main module name, hence we get collisions and why testing intermittently passes.

…d PCM clang modele re-scan

swiftlang/swift#33569 introduced a batch dependency scanning mode to save up on the overhead of clang instance creation.
This patch switches `swift-driver` over to using the new batch mode instead of generating individual `-scan-clang-dependencies` jobs for each re-scan.
@artemcm artemcm force-pushed the BatchClangScanAgain branch from 705f7c6 to b1b1d62 Compare September 4, 2020 18:31
@artemcm
Copy link
Contributor Author

artemcm commented Sep 4, 2020

@swift-ci please test

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented Sep 4, 2020

@swift-ci please test

@owenv
Copy link
Contributor

owenv commented Sep 5, 2020

Across different tests, instances of Driver share a temporary directory that we get from TSCBasic, because the path to the temporary directory is cached as a global variable.

That's some impressive debugging 😄 . Could this cause a problem for SPM builds with multiple targets as well, since it creates multiple drivers? Maybe each driver should create its own subdirectory for temporary files.

@artemcm
Copy link
Contributor Author

artemcm commented Sep 5, 2020

Across different tests, instances of Driver share a temporary directory that we get from TSCBasic, because the path to the temporary directory is cached as a global variable.

That's some impressive debugging 😄 . Could this cause a problem for SPM builds with multiple targets as well, since it creates multiple drivers? Maybe each driver should create its own subdirectory for temporary files.

Yeah, I thought about this case as well, but I do not think we can run into this scenario with SPM because it does not allow targets with the same name, and we use the target name to create this file. Furthermore, we would need to be using these Driver instances concurrently to run into trouble. Still, I think it might be a good idea to have each driver instance have its own temporary subdirectory, yeah.

@artemcm
Copy link
Contributor Author

artemcm commented Sep 8, 2020

@swift-ci please test

@artemcm artemcm requested a review from DougGregor September 8, 2020 16:25
@artemcm artemcm merged commit a065fa9 into swiftlang:master Sep 8, 2020
@artemcm artemcm deleted the BatchClangScanAgain branch January 20, 2021 19:00
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