================ @@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr<const CachedFileSystemEntry &> -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr<llvm::vfs::Status> Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( + StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr<llvm::vfs::Status> Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { - if (!shouldCacheStatFailures(Filename)) + if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto &Entry = - getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); - return insertLocalEntryForFilename(Filename, Entry); + getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); + return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) - return insertLocalEntryForFilename(Filename, *Entry); + return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto &UIDEntry = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return &getOrInsertSharedEntryForFilename(Filename, UIDEntry); + return &getOrInsertSharedEntryForFilename(FilenameForLookup, UIDEntry); } - return &getOrEmplaceSharedEntryForFilename(Filename, TEntry.getError()); + return &getOrEmplaceSharedEntryForFilename(FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr<EntryRef> DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( - StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) - return scanForDirectivesIfNecessary(*Entry, Filename, + StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { + FilenameForLookup = OriginalFilename; + } else { + PathBuf = WorkingDirForCacheLookup; + llvm::sys::path::append(PathBuf, OriginalFilename); ---------------- akyrtzi wrote:
Let's say that a filename gets passed in that is like this: "./whatever/./something/t.h" We make it absolute and then call `remove_dots` and it turns into this for the cache lookup: "/cwd/whatever/something/t.h" My intuition is that if a dot exists in the middle of a relative filename, it will likely also be there if clang forms an absolute path using it. So later let's say clang passes this filename to the function: "/cwd/whatever/./something/t.h" Because this is absolute we use it directly for the cache lookup, but this is different than the "dot-less" version so it's a cache miss. My point is if we call `remove_dots` when we absolutize a filename, in order to get a cache hit in a later call where clang passes the associated file as absolute filename, in practice it will defeat the purpose if a dot exists in the middle of the filename (it will be a cache miss), so it may be more profitable (more cache hits) to just remove the `./` prefix. The above is with the assumption that we do nothing to the filename if it is already absolute, are you both suggesting to process the filename and always call `remove_dots` whether the given filename is absolute or not? https://github.com/llvm/llvm-project/pull/66122 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits