Skip to content

Commit 5de0144

Browse files
ChuanqiXu9ChuanqiXu9
committed
Update
1 parent 42eb382 commit 5de0144

File tree

2 files changed

+131
-78
lines changed

2 files changed

+131
-78
lines changed

‎clang-tools-extra/clangd/ModulesBuilder.cpp‎

Lines changed: 73 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -357,74 +357,51 @@ void ModuleFileCache::remove(StringRef ModuleName) {
357357
ModuleFiles.erase(ModuleName);
358358
}
359359

360-
/// Collect the directly and indirectly required module names for \param
361-
/// ModuleName in topological order. The \param ModuleName is guaranteed to
362-
/// be the last element in \param ModuleNames.
363-
llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource,
364-
ProjectModules &MDB,
365-
StringRef ModuleName) {
366-
llvm::SmallVector<llvm::StringRef> ModuleNames;
367-
llvm::StringSet<> ModuleNamesSet;
368-
369-
auto VisitDeps = [&](StringRef ModuleName, auto Visitor) -> void {
370-
ModuleNamesSet.insert(ModuleName);
360+
class ModuleNameToSourceCache {
361+
public:
362+
std::string getSourceForModuleName(llvm::StringRef ModuleName) {
363+
std::lock_guard<std::mutex> Lock(CacheMutex);
364+
auto Iter = ModuleNameToSourceCache.find(ModuleName);
365+
if (Iter != ModuleNameToSourceCache.end())
366+
return Iter->second;
367+
return "";
368+
}
371369

372-
for (StringRef RequiredModuleName : MDB.getRequiredModules(
373-
MDB.getSourceForModuleName(ModuleName, RequiredSource)))
374-
if (ModuleNamesSet.insert(RequiredModuleName).second)
375-
Visitor(RequiredModuleName, Visitor);
370+
void addEntry(llvm::StringRef ModuleName, PathRef Source) {
371+
std::lock_guard<std::mutex> Lock(CacheMutex);
372+
ModuleNameToSourceCache[ModuleName] = Source.str();
373+
}
376374

377-
ModuleNames.push_back(ModuleName);
378-
};
379-
VisitDeps(ModuleName, VisitDeps);
375+
void eraseEntry(llvm::StringRef ModuleName) {
376+
std::lock_guard<std::mutex> Lock(CacheMutex);
377+
ModuleNameToSourceCache.erase(ModuleName);
378+
}
380379

381-
return ModuleNames;
382-
}
380+
private:
381+
std::mutex CacheMutex;
382+
llvm::StringMap<std::string> ModuleNameToSourceCache;
383+
};
383384

384385
class CachingProjectModules : public ProjectModules {
385386
public:
386-
CachingProjectModules(const GlobalCompilationDatabase &CDB) : CDB(CDB) {}
387+
CachingProjectModules(std::unique_ptr<ProjectModules> MDB,
388+
ModuleNameToSourceCache &Cache)
389+
: MDB(std::move(MDB)), Cache(Cache) {
390+
assert(this->MDB && "CachingProjectModules should only be created with a "
391+
"valid underlying ProjectModules");
392+
}
387393

388394
std::vector<std::string> getRequiredModules(PathRef File) override {
389-
std::unique_ptr<ProjectModules> MDB = CDB.getProjectModules(File);
390-
if (!MDB) {
391-
elog("Failed to get Project Modules information for {0}", File);
392-
return {};
393-
}
394395
return MDB->getRequiredModules(File);
395396
}
396397

397398
std::string getModuleNameForSource(PathRef File) override {
398-
std::unique_ptr<ProjectModules> MDB = CDB.getProjectModules(File);
399-
if (!MDB) {
400-
elog("Failed to get Project Modules information for {0}", File);
401-
return {};
402-
}
403399
return MDB->getModuleNameForSource(File);
404400
}
405401

406-
void setCommandMangler(CommandMangler M) override {
407-
// GlobalCompilationDatabase::getProjectModules() will set mangler
408-
// for the underlying ProjectModules.
409-
}
410-
411402
std::string getSourceForModuleName(llvm::StringRef ModuleName,
412403
PathRef RequiredSrcFile) override {
413-
std::string CachedResult;
414-
{
415-
std::lock_guard<std::mutex> Lock(CacheMutex);
416-
auto Iter = ModuleNameToSourceCache.find(ModuleName);
417-
if (Iter != ModuleNameToSourceCache.end())
418-
CachedResult = Iter->second;
419-
}
420-
421-
std::unique_ptr<ProjectModules> MDB =
422-
CDB.getProjectModules(RequiredSrcFile);
423-
if (!MDB) {
424-
elog("Failed to get Project Modules information for {0}",
425-
RequiredSrcFile);
426-
return {};
427-
}
404+
std::string CachedResult = Cache.getSourceForModuleName(ModuleName);
428405

429406
// Verify Cached Result by seeing if the source declaring the same module
430407
// as we query.
@@ -433,57 +410,70 @@ class CachingProjectModules : public ProjectModules {
433410
MDB->getModuleNameForSource(CachedResult);
434411
if (ModuleNameOfCachedSource == ModuleName)
435412
return CachedResult;
436-
else {
437-
// Cached Result is invalid. Clear it.
438413

439-
std::lock_guard<std::mutex> Lock(CacheMutex);
440-
ModuleNameToSourceCache.erase(ModuleName);
441-
}
414+
// Cached Result is invalid. Clear it.
415+
Cache.eraseEntry(ModuleName);
442416
}
443417

444418
auto Result = MDB->getSourceForModuleName(ModuleName, RequiredSrcFile);
445-
446-
{
447-
std::lock_guard<std::mutex> Lock(CacheMutex);
448-
ModuleNameToSourceCache.insert({ModuleName, Result});
449-
}
419+
Cache.addEntry(ModuleName, Result);
450420

451421
return Result;
452422
}
453423

454424
private:
455-
const GlobalCompilationDatabase &CDB;
456-
457-
std::mutex CacheMutex;
458-
llvm::StringMap<std::string> ModuleNameToSourceCache;
425+
std::unique_ptr<ProjectModules> MDB;
426+
ModuleNameToSourceCache &Cache;
459427
};
460428

429+
/// Collect the directly and indirectly required module names for \param
430+
/// ModuleName in topological order. The \param ModuleName is guaranteed to
431+
/// be the last element in \param ModuleNames.
432+
llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource,
433+
CachingProjectModules &MDB,
434+
StringRef ModuleName) {
435+
llvm::SmallVector<llvm::StringRef> ModuleNames;
436+
llvm::StringSet<> ModuleNamesSet;
437+
438+
auto VisitDeps = [&](StringRef ModuleName, auto Visitor) -> void {
439+
ModuleNamesSet.insert(ModuleName);
440+
441+
for (StringRef RequiredModuleName : MDB.getRequiredModules(
442+
MDB.getSourceForModuleName(ModuleName, RequiredSource)))
443+
if (ModuleNamesSet.insert(RequiredModuleName).second)
444+
Visitor(RequiredModuleName, Visitor);
445+
446+
ModuleNames.push_back(ModuleName);
447+
};
448+
VisitDeps(ModuleName, VisitDeps);
449+
450+
return ModuleNames;
451+
}
452+
461453
} // namespace
462454

463455
class ModulesBuilder::ModulesBuilderImpl {
464456
public:
465-
ModulesBuilderImpl(const GlobalCompilationDatabase &CDB)
466-
: CachedProjectModules(CDB), Cache(CDB) {}
457+
ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) : Cache(CDB) {}
467458

468-
CachingProjectModules &getCachedProjectModules() {
469-
return CachedProjectModules;
459+
ModuleNameToSourceCache &getProjectModulesCache() {
460+
return ProjectModulesCache;
470461
}
471-
472462
const GlobalCompilationDatabase &getCDB() const { return Cache.getCDB(); }
473463

474464
llvm::Error
475465
getOrBuildModuleFile(PathRef RequiredSource, StringRef ModuleName,
476-
const ThreadsafeFS &TFS, ProjectModules &MDB,
466+
const ThreadsafeFS &TFS, CachingProjectModules &MDB,
477467
ReusablePrerequisiteModules &BuiltModuleFiles);
478468

479469
private:
480-
CachingProjectModules CachedProjectModules;
481470
ModuleFileCache Cache;
471+
ModuleNameToSourceCache ProjectModulesCache;
482472
};
483473

484474
llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
485475
PathRef RequiredSource, StringRef ModuleName, const ThreadsafeFS &TFS,
486-
ProjectModules &MDB, ReusablePrerequisiteModules &BuiltModuleFiles) {
476+
CachingProjectModules &MDB, ReusablePrerequisiteModules &BuiltModuleFiles) {
487477
if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
488478
return llvm::Error::success();
489479

@@ -534,17 +524,24 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
534524
std::unique_ptr<PrerequisiteModules>
535525
ModulesBuilder::buildPrerequisiteModulesFor(PathRef File,
536526
const ThreadsafeFS &TFS) {
537-
ProjectModules &MDB = Impl->getCachedProjectModules();
527+
std::unique_ptr<ProjectModules> MDB = Impl->getCDB().getProjectModules(File);
528+
if (!MDB) {
529+
elog("Failed to get Project Modules information for {0}", File);
530+
return std::make_unique<FailedPrerequisiteModules>();
531+
}
532+
CachingProjectModules CachedMDB(std::move(MDB),
533+
Impl->getProjectModulesCache());
538534

539-
std::vector<std::string> RequiredModuleNames = MDB.getRequiredModules(File);
535+
std::vector<std::string> RequiredModuleNames =
536+
CachedMDB.getRequiredModules(File);
540537
if (RequiredModuleNames.empty())
541538
return std::make_unique<ReusablePrerequisiteModules>();
542539

543540
auto RequiredModules = std::make_unique<ReusablePrerequisiteModules>();
544541
for (llvm::StringRef RequiredModuleName : RequiredModuleNames) {
545542
// Return early if there is any error.
546543
if (llvm::Error Err = Impl->getOrBuildModuleFile(
547-
File, RequiredModuleName, TFS, MDB, *RequiredModules.get())) {
544+
File, RequiredModuleName, TFS, CachedMDB, *RequiredModules.get())) {
548545
elog("Failed to build module {0}; due to {1}", RequiredModuleName,
549546
toString(std::move(Err)));
550547
return std::make_unique<FailedPrerequisiteModules>();

‎clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,54 @@
2727
namespace clang::clangd {
2828
namespace {
2929

30+
class GlobalScanningCounterProjectModules : public ProjectModules {
31+
public:
32+
GlobalScanningCounterProjectModules(
33+
std::unique_ptr<ProjectModules> Underlying, std::atomic<unsigned> &Count)
34+
: Underlying(std::move(Underlying)), Count(Count) {}
35+
36+
std::vector<std::string> getRequiredModules(PathRef File) override {
37+
return Underlying->getRequiredModules(File);
38+
}
39+
40+
std::string getModuleNameForSource(PathRef File) override {
41+
return Underlying->getModuleNameForSource(File);
42+
}
43+
44+
void setCommandMangler(CommandMangler Mangler) override {
45+
Underlying->setCommandMangler(std::move(Mangler));
46+
}
47+
48+
std::string getSourceForModuleName(llvm::StringRef ModuleName,
49+
PathRef RequiredSrcFile) override {
50+
Count++;
51+
return Underlying->getSourceForModuleName(ModuleName, RequiredSrcFile);
52+
}
53+
54+
private:
55+
std::unique_ptr<ProjectModules> Underlying;
56+
std::atomic<unsigned> &Count;
57+
};
58+
3059
class MockDirectoryCompilationDatabase : public MockCompilationDatabase {
3160
public:
3261
MockDirectoryCompilationDatabase(StringRef TestDir, const ThreadsafeFS &TFS)
3362
: MockCompilationDatabase(TestDir),
3463
MockedCDBPtr(std::make_shared<MockClangCompilationDatabase>(*this)),
35-
TFS(TFS) {
64+
TFS(TFS), GlobalScanningCount(0) {
3665
this->ExtraClangFlags.push_back("-std=c++20");
3766
this->ExtraClangFlags.push_back("-c");
3867
}
3968

4069
void addFile(llvm::StringRef Path, llvm::StringRef Contents);
4170

4271
std::unique_ptr<ProjectModules> getProjectModules(PathRef) const override {
43-
return scanningProjectModules(MockedCDBPtr, TFS);
72+
return std::make_unique<GlobalScanningCounterProjectModules>(
73+
scanningProjectModules(MockedCDBPtr, TFS), GlobalScanningCount);
4474
}
4575

76+
unsigned getGlobalScanningCount() const { return GlobalScanningCount; }
77+
4678
private:
4779
class MockClangCompilationDatabase : public tooling::CompilationDatabase {
4880
public:
@@ -68,6 +100,8 @@ class MockDirectoryCompilationDatabase : public MockCompilationDatabase {
68100

69101
std::shared_ptr<MockClangCompilationDatabase> MockedCDBPtr;
70102
const ThreadsafeFS &TFS;
103+
104+
mutable std::atomic<unsigned> GlobalScanningCount;
71105
};
72106

73107
// Add files to the working testing directory and the compilation database.
@@ -590,6 +624,28 @@ export constexpr int M = 43;
590624
EXPECT_NE(NewHSOptsA.PrebuiltModuleFiles, HSOptsA.PrebuiltModuleFiles);
591625
}
592626

627+
TEST_F(PrerequisiteModulesTests, ScanningCacheTest) {
628+
MockDirectoryCompilationDatabase CDB(TestDir, FS);
629+
630+
CDB.addFile("M.cppm", R"cpp(
631+
export module M;
632+
)cpp");
633+
CDB.addFile("A.cppm", R"cpp(
634+
export module A;
635+
import M;
636+
)cpp");
637+
CDB.addFile("B.cppm", R"cpp(
638+
export module B;
639+
import M;
640+
)cpp");
641+
642+
ModulesBuilder Builder(CDB);
643+
644+
Builder.buildPrerequisiteModulesFor(getFullPath("A.cppm"), FS);
645+
Builder.buildPrerequisiteModulesFor(getFullPath("B.cppm"), FS);
646+
EXPECT_EQ(CDB.getGlobalScanningCount(), 1u);
647+
}
648+
593649
} // namespace
594650
} // namespace clang::clangd
595651

0 commit comments

Comments
 (0)