From c598a4609c103aa94b08fc2537265caf77a96e81 Mon Sep 17 00:00:00 2001 From: Jochem van Boxtel Date: Sat, 1 Jun 2024 16:07:23 +0200 Subject: [PATCH] cmFileSet: Fix poor performance of large file sets with export() on Windows Exporting targets having large FILE_SETs with install(EXPORT) or export(EXPORT) currently performs poorly on Windows compared to Unix-like systems, because cmFileSet::EvaluateDirectoryEntries calls SystemTools::SameFile on every pair of parent directories in the file set. SystemTools::SameFile opens and closes two read-only filesystem handles. This causes a significant performance drop on Windows for FILE_SETs with even a couple of dozens of files. Use the recently added SystemTools::GetFileId function in kwsys (https://gitlab.kitware.com/utils/kwsys/-/merge_requests/298) instead of SameFile to cache the identity of a directory in cmFileSet::EvaluateDirectoryEntries. This means only one filesystem handle is needed per distinct directory path, instead of two per (even if they're equal) directory pair. --- Source/cmFileSet.cxx | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/Source/cmFileSet.cxx b/Source/cmFileSet.cxx index b74855f212..a00c10e6fd 100644 --- a/Source/cmFileSet.cxx +++ b/Source/cmFileSet.cxx @@ -4,9 +4,11 @@ #include #include +#include #include #include +#include #include #include @@ -158,6 +160,13 @@ std::vector cmFileSet::EvaluateDirectoryEntries( const cmGeneratorTarget* target, cmGeneratorExpressionDAGChecker* dagChecker) const { + struct DirCacheEntry + { + std::string collapsedDir; + cm::optional fileId; + }; + + std::unordered_map dirCache; std::vector result; for (auto const& cge : cges) { auto entry = cge->Evaluate(lg, config, target, dagChecker); @@ -166,12 +175,29 @@ std::vector cmFileSet::EvaluateDirectoryEntries( if (!cmSystemTools::FileIsFullPath(dir)) { dir = cmStrCat(lg->GetCurrentSourceDirectory(), '/', dir); } - auto collapsedDir = cmSystemTools::CollapseFullPath(dir); + + auto dirCacheResult = dirCache.emplace(dir, DirCacheEntry()); + auto& dirCacheEntry = dirCacheResult.first->second; + const auto isNewCacheEntry = dirCacheResult.second; + + if (isNewCacheEntry) { + cmSystemTools::FileId fileId; + auto isFileIdValid = cmSystemTools::GetFileId(dir, fileId); + dirCacheEntry.collapsedDir = cmSystemTools::CollapseFullPath(dir); + dirCacheEntry.fileId = + isFileIdValid ? cm::optional(fileId) : cm::nullopt; + } + for (auto const& priorDir : result) { - auto collapsedPriorDir = cmSystemTools::CollapseFullPath(priorDir); - if (!cmSystemTools::SameFile(collapsedDir, collapsedPriorDir) && - (cmSystemTools::IsSubDirectory(collapsedDir, collapsedPriorDir) || - cmSystemTools::IsSubDirectory(collapsedPriorDir, collapsedDir))) { + auto priorDirCacheEntry = dirCache.at(priorDir); + bool sameFile = dirCacheEntry.fileId.has_value() && + priorDirCacheEntry.fileId.has_value() && + (*dirCacheEntry.fileId == *priorDirCacheEntry.fileId); + if (!sameFile && + (cmSystemTools::IsSubDirectory(dirCacheEntry.collapsedDir, + priorDirCacheEntry.collapsedDir) || + cmSystemTools::IsSubDirectory(priorDirCacheEntry.collapsedDir, + dirCacheEntry.collapsedDir))) { lg->GetCMakeInstance()->IssueMessage( MessageType::FATAL_ERROR, cmStrCat(