Skip to content

[Explicit Module Builds] Introduce an InterModuleDependencyOracle abstraction for tracking and sharing dependency scanning results #356

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Dec 15, 2020
Merged

[Explicit Module Builds] Introduce an InterModuleDependencyOracle abstraction for tracking and sharing dependency scanning results #356

merged 8 commits into from
Dec 15, 2020

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Nov 11, 2020 •
edited
Loading

This PR refactors the handling of inter-module dependencies to centralize their handling in a new, reference-type, oracle.

InterModuleDependencyOracle will now be responsible for responding to queries about dependencies of a given module.
The intent behind it is to abstract the actual mechanism used for tracking/computing/caching inter-module dependencies. A single oracle reference is intended to be passed around to multiple Driver invocations, corresponding to different targets, by build-system clients (e.g. SwiftPM), replacing today's use of ModuleInfoMap-based API, to communicate already-computed dependency-scan results.

As implemented, it is a simple store of ModuleInfo value objects, which a Driver invocation populates by invoking a -scan-dependencies job; not unlike passing around an accumulating ModuleInfoMap.
In the future, the oracle's internal implementation may be replaced with a direct dependency-scanner library access, without having to break the clients' API.

There will be a followup PR to cleanup some of the shims required to ensure that this does not break the API currently used by SwiftPM.

Corresponding PR to adapt this API in SwiftPM: swiftlang/swift-package-manager#3042

Resolves rdar://71209183

@artemcm
Copy link
Contributor Author

artemcm commented Nov 11, 2020

@swift-ci please test

This was referenced Nov 11, 2020
Closed
Merged
cltnschlosser
cltnschlosser reviewed Nov 11, 2020
View reviewed changes
Copy link
Contributor

@cltnschlosser cltnschlosser left a comment

Choose a reason for hiding this comment

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

Seems like a clear win for users of libSwiftDriver (SPM).

Any negative here when trying to do explicit module builds solely from the command line? Didn't seem like it, but I don't know enough about how this works to tell.

@artemcm
Copy link
Contributor Author

artemcm commented Nov 11, 2020

Thank you for taking a look, @cltnschlosser, and for your suggestions.

Any negative here when trying to do explicit module builds solely from the command line? Didn't seem like it, but I don't know enough about how this works to tell.

This does not make a difference to command line builds today; but, it does open up a possibility for the future to no longer have the driver launch -scan-dependencies jobs, instead talking to the dependency scanner directly via a library that the oracle will wrap.

@artemcm artemcm mentioned this pull request Nov 11, 2020
Closed
@artemcm
Copy link
Contributor Author

artemcm commented Nov 11, 2020

@swift-ci please test

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented Nov 11, 2020

@swift-ci please test

@artemcm artemcm marked this pull request as ready for review November 11, 2020 18:43
@artemcm
Copy link
Contributor Author

artemcm commented Dec 3, 2020

@swift-ci please test

@artemcm
Copy link
Contributor Author

artemcm commented Dec 10, 2020

@DougGregor @nkcsgexi ping for a review.
This PR includes a bunch of shims/adaptors to ensure that the API currently in-use by SwiftPM continues working, but it would be nice to get this in to help with the transition to:
swiftlang/swift#34786

@artemcm
Copy link
Contributor Author

artemcm commented Dec 10, 2020

@swift-ci please test

DougGregor
DougGregor approved these changes Dec 11, 2020
View reviewed changes
...iftDriver/Explicit Module Builds/Inter Module Dependencies/InterModuleDependencyOracle.swift
/// An abstraction of a cache and query-engine of inter-module dependencies
public class InterModuleDependencyOracle {
/// Query the ModuleInfo of a module with a given ID
@_spi(Testing) public func getModuleInfo(of moduleId: ModuleDependencyId) -> ModuleInfo? {
Copy link
Member

Choose a reason for hiding this comment

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

Should this class be made thread-safe? Clients may very well have multiple threads in which they create Driver instances that reference the same oracle...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! I've added some basic thread-safety to it now, and will iterate on it as this type transitions to wrapping libSwiftScan.

@artemcm
[Explicit Module Builds] Introduce an InterModuleDependencyOracle abs…
9a42dec 
…traction for tracking and sharing dependency scanning results

This PR refactors the handling of inter-module dependencies to centralize their handling in a new, reference-type, oracle.

`InterModuleDependencyOracle` will now be responsible for responding to queries about dependencies of a given module.
The intent behind it is to abstract the actual mechanism used for tracking/computing/caching inter-module dependencies. A single oracle reference is intended to be passed around to multiple Driver invocations, corresponding to different targets, by build-system clients (e.g. SwiftPM), replacing today's use of `ModuleInfoMap`-based API, to communicate already-computed dependency-scan results.

As implemented, it is a simple store of `ModuleInfo` value objects, which a `Driver` invocation populates by invoking a `-scan-dependencies` job; not unlike passing around an accumulating `ModuleInfoMap`.
In the future, the oracle's internal implementation may be replaced with a direct dependency-scanner library access, without having to break the clients' API.
@artemcm
[Explicit Module Builds] Add backwards-compatibility shim for depende…
84e4983 
…ncy graph getter
@artemcm
[Explicit Module Builds] Refactor placeholder dependency resolution t…
d98083c 
…o use the dependency oracle
@artemcm
[Explicit Module Builds] Revert the explicit dependency build planner…
fa1c4db 
… to operate on dependency graphs instead of querying the oracle
@artemcm
[Explicit Module Builds] Add extra adapter to handle previous ModuleI…
03e994b 
…nfoMap-based API used by SwiftPM
@artemcm
Code cleanup according to code-review feedback
47c943d
@artemcm
[Explicit Module Builds] Add another shim for backwards-compatibility…
abc4959 
… with API currently used by SwiftPM to communicate external build artifacts
@artemcm
[Explicit Module Builds] Add basic thread-safety to InterModuleDepend…
cc413d8 
…encyOracle
@artemcm
Copy link
Contributor Author

artemcm commented Dec 14, 2020

@swift-ci please test

@artemcm artemcm mentioned this pull request Dec 14, 2020
Closed
@artemcm artemcm merged commit 7055a62 into swiftlang:main Dec 15, 2020
@artemcm artemcm deleted the DependencyOracle branch January 20, 2021 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cltnschlosser cltnschlosser cltnschlosser left review comments

@DougGregor DougGregor DougGregor approved these changes

@nkcsgexi nkcsgexi Awaiting requested review from nkcsgexi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@artemcm @DougGregor @cltnschlosser