Skip to content

[Dependency Scanning] Split the ModuleDependencyCache into two: current scan cache & global. #38761

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
Aug 6, 2021

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Aug 5, 2021

This change causes the cache to be layered with a local "cache" that wraps the global cache, which will serve as the source of truth. The local cache persists only for the duration of a given scanning action, and has a store of references to dependencies resolved as a part of the current scanning action only, while the global cache is the one that persists across scanning actions (e.g. in DependencyScanningTool) and stores actual module dependency info values.

Only the local cache can answer dependency lookup queries, checking current scanning action results first, before falling back to querying the global cache, with queries disambiguated by the current scannning action's search paths, ensuring we never resolve a dependency lookup query with a module info that could not be found in the current action's search paths.

This change is required because search-path disambiguation can lead to false-negatives: for example, the Clang dependency scanner may find modules relative to the compiler's path that are not on the compiler's direct search paths. While such false-negative query responses should be functionally safe, we rely on the current scanning action's results being always-present-in-the-cache for the scanner's functionality. This layering ensures that the cache use-sites remain unchanged and that we get both: preserved global state which can be queried disambiguated with the search path details, and an always-consistent local (current action) cache state.

@artemcm artemcm requested a review from nkcsgexi August 5, 2021 17:27
@artemcm
Copy link
Contributor Author

artemcm commented Aug 5, 2021

@swift-ci please test

@artemcm
Copy link
Contributor Author

artemcm commented Aug 5, 2021

@swift-ci please test Windows platform

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.

Thanks! Some comments are inline.

@artemcm
Copy link
Contributor Author

artemcm commented Aug 5, 2021

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Aug 5, 2021

Build failed
Swift Test Linux Platform
Git Sha - 5e33867388ea9a9549e81aed0814ed427faeaf74

@artemcm
Copy link
Contributor Author

artemcm commented Aug 5, 2021

@swift-ci please test

…nt scan cache & global.

This change causes the cache to be layered with a local "cache" that wraps the global cache, which will serve as the source of truth. The local cache persists only for the duration of a given scanning action, and has a store of references to dependencies resolved as a part of the current scanning action only, while the global cache is the one that persists across scanning actions (e.g. in `DependencyScanningTool`) and stores actual module dependency info values.

Only the local cache can answer dependency lookup queries, checking current scanning action results first, before falling back to querying the global cache, with queries disambiguated by the current scannning action's search paths, ensuring we never resolve a dependency lookup query with a module info that could not be found in the current action's search paths.

This change is required because search-path disambiguation can lead to false-negatives: for example, the Clang dependency scanner may find modules relative to the compiler's path that are not on the compiler's direct search paths. While such false-negative query responses should be functionally safe, we rely on the current scanning action's results being always-present-in-the-cache for the scanner's functionality. This layering ensures that the cache use-sites remain unchanged and that we get both: preserved global state which can be queried disambiguated with the search path details, and an always-consistent local (current action) cache state.
@artemcm
Copy link
Contributor Author

artemcm commented Aug 6, 2021

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

LGTM!

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