Skip to content

[Dependency Scanning] Bridge Clang dependency scanner results on-demand #83600

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Aug 7, 2025

#83050 changed the behavior to query every Clang module identifier imported from Swift, even for modules which may have previously been discovered as transitive dependencies of another query. This means we now have a lot of results coming in from Clang scanner queries which are redundant, and we bridge them all, all of the time, which adds up to a substantial amount of time.

Instead of always bridging all of the discovered modules of all of the queries, only do so for modules which have not already been cached at the time we record query results.

@artemcm
Copy link
Contributor Author

artemcm commented Aug 7, 2025

@swift-ci smoke test

@artemcm artemcm force-pushed the NoRedundantClangDepBridging branch from f345188 to 4876b2f Compare August 7, 2025 22:28
@artemcm
Copy link
Contributor Author

artemcm commented Aug 7, 2025

@swift-ci smoke test

Instead of always bridging all of the discovered modules of all of the queries, only do so for modules which are not already cached
@artemcm artemcm force-pushed the NoRedundantClangDepBridging branch from 4876b2f to 8cf2714 Compare August 12, 2025 16:10
@artemcm
Copy link
Contributor Author

artemcm commented Aug 12, 2025

@swift-ci smoke test

Copy link
Contributor

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

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

Function wise it is all good. Just some comments about the APIs.

clang::tooling::dependencies::ModuleOutputKind)>;

ModuleDependencyInfo
bridgeClangModuleDependency(
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be a static method as implementation details.

Or maybe it can be a private method in DependencyScanner? The callbacks all have a capture of the scanner reference, which is probably fine since scanner should outlive everything but if the bridging callbacks are part of the scanner, then there is no need for callback or worry about lifetimes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This required a bigger refactor than it originally seemed, but I moved the bridging code into a private scanner utility.

…leDependencyScanner' utility

This moves the functionality of 'bridgeClangModuleDependency' into a utility in the main scanner class because it relies on various objects whose lifetime is already tied to the scanner itself.
@artemcm
Copy link
Contributor Author

artemcm commented Aug 13, 2025

@swift-ci smoke test

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented Aug 13, 2025

@swift-ci smoke test

@artemcm
Copy link
Contributor Author

artemcm commented Aug 13, 2025

@swift-ci smoke test Linux platform

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.

2 participants