Skip to content

[5.9][Dependency Scanning] Isolate shared dependency scanner state #64652

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
Mar 28, 2023

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Mar 27, 2023

Cherry-pick of #64615

Using mutual exclusion, ensuring that multiple threads executing dependency scans do not encounter data races on shared mutable state.

There are two layers with shared state where we need to be careful:

  • DependencyScanningTool, as the main entity that scanning clients interact with. This tool instantiates compiler instances for individual scans, computing a scanning invocation hash. It needs to remember those instances for future use, and when creating instances it needs to reset LLVM argument processor's global state, meaning all uses of argument processing must be in a critical section.

  • SwiftDependencyScanningService, as the main cache where dependency scanning results are stored. Each individual scan instantiates a ModuleDependenciesCache, which uses the scanning service as the underlying storage. The services' storage is segmented to storing dependencies discovered in a scan with a given context hash, which means two different scanning invocations running at the same time will be accessing different locations in its storage, thus not requiring synchronization. But the service still has some shared state that must be protected, such as the collection of discovered source modules, and the map used to query context-hash-specific underlying cache storage.

Using mutual exclusion, ensuring that multiple threads executing dependency scans do not encounter data races on shared mutable state.

There are two layers with shared state where we need to be careful:
- `DependencyScanningTool`, as the main entity that scanning clients interact with. This tool instantiates compiler instances for individual scans, computing a scanning invocation hash. It needs to remember those instances for future use, and when creating instances it needs to reset LLVM argument processor's global state, meaning all uses of argument processing must be in a critical section.

- `SwiftDependencyScanningService`, as the main cache where dependency scanning results are stored. Each individual scan instantiates a `ModuleDependenciesCache`, which uses the scanning service as the underlying storage. The services' storage is segmented to storing dependencies discovered in a scan with a given context hash, which means two different scanning invocations running at the same time will be accessing different locations in its storage, thus not requiring synchronization. But the service still has some shared state that must be protected, such as the collection of discovered source modules, and the map used to query context-hash-specific underlying cache storage.
@artemcm artemcm requested a review from nkcsgexi March 27, 2023 21:13
@artemcm artemcm requested a review from a team as a code owner March 27, 2023 21:13
@artemcm
Copy link
Contributor Author

artemcm commented Mar 27, 2023

@swift-ci smoke test

Copy link
Contributor

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

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

Dependency scanner change has no impacts on implicit module build.

@artemcm
Copy link
Contributor Author

artemcm commented Mar 28, 2023

@swift-ci test

@artemcm artemcm merged commit 6df1237 into swiftlang:release/5.9 Mar 28, 2023
@artemcm artemcm deleted the 59ProtecSharedScannerState branch March 28, 2023 19:44
@AnthonyLatsis AnthonyLatsis added the 🍒 release cherry pick Flag: Release branch cherry picks label May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants