[Dependency Scanning][Refactor] Reduce the number of ClangImporter instances used by the scanner
#81361
Conversation
|
@swift-ci test |
artemcm
force-pushed
the
DepScanFewerClangs
branch
from
634bafc to
92632e6
Compare
|
@swift-ci test |
… scanning worker Move relevant logic directly into the worker
artemcm
force-pushed
the
DepScanFewerClangs
branch
from
92632e6 to
5b13c94
Compare
|
|
||
| // When caching is enabled, we rely on ClangImporter for | ||
| // 'addClangInvovcationDependencies' | ||
| if (Invocation.getCASOptions().EnableCaching) { |
There was a problem hiding this comment.
cc @cachemeifyoucan
This is due to:
swift/lib/Serialization/ScanningLoaders.cpp
Line 248 in df32362
I imagine we ought to be able to factor that out to something that happens once at worker creation, too.
There was a problem hiding this comment.
I have a feeling that I might be able to remove that function now. Let me check (Oh, there was a bad typo in function name)
There was a problem hiding this comment.
Here is a draft: #81389
I need to do more clean up before commit that PR, will comment in that PR about background
There was a problem hiding this comment.
Thanks! Could I get a review of this change in the meantime? :)
We can land #81389 as a follow-up.
|
@swift-ci test |
|
@swift-ci test Linux platform |
artemcm
requested review from
xymus,
hborla,
slavapestov,
xedin,
tshortli,
zoecarver,
egorzhdan,
beccadax,
ian-twilightcoder,
Xazax-hun,
j-hui,
fahadnayyar and
susmonteiro
as code owners
| Args.push_back("-Xcc"); | ||
| Args.push_back("-fno-implicit-module-maps"); | ||
| } | ||
| for (const auto &arg : swiftModuleClangCC1CommandLineArgs) |
There was a problem hiding this comment.
Nit: llvm::append_range(Args, swiftModuleClangCC1CommandLineArgs);
| // with `-direct-clang-cc1-module-build`. | ||
| if (ScanASTContext.ClangImporterOpts.ClangImporterDirectCC1Scan) { | ||
| swiftModuleClangCC1CommandLineArgs.push_back("-direct-clang-cc1-module-build"); | ||
| auto *importer = |
There was a problem hiding this comment.
This importer is not different from scanClangImporter above right?
| ModuleDependencyScanningWorker::ModuleDependencyScanningWorker( | ||
| SwiftDependencyScanningService &globalScanningService, | ||
| const CompilerInvocation &ScanCompilerInvocation, | ||
| const SILOptions &SILOptions, ASTContext &ScanASTContext, | ||
| swift::DependencyTracker &DependencyTracker, DiagnosticEngine &Diagnostics) | ||
| : clangScanningTool(*globalScanningService.ClangScanningService, | ||
| globalScanningService.getClangScanningFS()) { | ||
| globalScanningService.getClangScanningFS()), | ||
| swiftModuleClangCC1CommandLineArgs() { |
There was a problem hiding this comment.
This default initialization is not needed.
…r textual interface scanning sub-instance
artemcm
force-pushed
the
DepScanFewerClangs
branch
from
5b13c94 to
80c71fd
Compare
|
@swift-ci test |
…langs" This reverts commit 8ab726c.
This is a refactoring PR that includes two changes meant to reduce the overall number of times that
ClangImporteris instantiated during a given dependency scanning action:Remove 'ClangImporter' instance from dependency scanning worker
This change moves
getModuleDependencyandgetHeaderDependencyfunctionality fromClangImporterdirectly into Swift'sModuleDependencyScanningWorkerusing theclangScanningToolit already has, removing the need to instantiate an instance ofClangImporterper-worker.Reduce number of module loaders instantiated for textual interface scanning sub-instance
When creating a compilation sub-instance for scanning a textual Swift module interface for dependencies, in
setupModuleLoadersonly configure the strictly-necessary module loaders, skippingClangImporterin particular.On a large benchmark scan (e.g. importing every Swift module in the macOS SDK yielding ~270 Swift modules and ~530 Clang modules) this change results in a ~5% speedup.