[PATCH] D114966: [clang][deps] Split stat and file content caches

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114966/new/ https://reviews.llvm.org/D114966 __

[PATCH] D114966: [clang][deps] Split stat and file content caches

jansvoboda11 added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:132-134 + /// Non-owning pointer to the file contents. This can be shared between + /// multiple entries (e.g. between a symlink and its target). + st

[PATCH] D114966: [clang][deps] Split stat and file content caches

jansvoboda11 updated this revision to Diff 401171. jansvoboda11 marked 7 inline comments as done. jansvoboda11 added a comment. Update comments, remove `std::atomic<>`, merge `getOrEmplaceContentsForUID` into `getOrEmplaceEntryForUID`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D114966: [clang][deps] Split stat and file content caches

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. Okay, I think this is the last round. Everything looks correct, except a few out-of-date comments. Two things, one small, one bigger (but not too big I think). Smaller one i

[PATCH] D114966: [clang][deps] Split stat and file content caches

jansvoboda11 updated this revision to Diff 395684. jansvoboda11 added a comment. Erase filenames in temporary `Stat` objects, use `UniqueID` as shard key where appropriate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114966/new/ https://reviews.

[PATCH] D114966: [clang][deps] Split stat and file content caches

jansvoboda11 added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:359-360 + findSharedEntryByUID(llvm::vfs::Status Stat) const { +return SharedCache.getShard(Stat.getName()) +.findEntryByUID(Stat.getUniqueI

[PATCH] D114966: [clang][deps] Split stat and file content caches

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. I'm liking the new direction here; requesting changes since it looks like the filename is being used to pick the shard for UIDMap, which will lead to multiple opinions of wha

[PATCH] D114966: [clang][deps] Split stat and file content caches

jansvoboda11 added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:275 + const auto &Entry1 = getOrEmplaceSharedEntryForUID(std::move(TEntry)); + const auto &Entry2 = getOrInsertSharedEntryForFilename(Filename, Entry1); + ret

[PATCH] D114966: [clang][deps] Split stat and file content caches

jansvoboda11 updated this revision to Diff 395103. jansvoboda11 added a comment. Re-run CI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114966/new/ https://reviews.llvm.org/D114966 Files: clang/include/clang/Tooling/DependencyScanning/Dependen

[PATCH] D114966: [clang][deps] Split stat and file content caches

jansvoboda11 updated this revision to Diff 395102. jansvoboda11 added a comment. Implement new version that ensures the stat and contents of a file are always in sync. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114966/new/ https://reviews.llvm.

[PATCH] D114966: [clang][deps] Split stat and file content caches

jansvoboda11 added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:200-210 + // FIXME: This read can fail. + // In that case, we should probably update `CacheEntry::MaybeStat`. + // However, that is co

[PATCH] D114966: [clang][deps] Split stat and file content caches

dexonsmith added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:110-111 + +CachedFileContents &DependencyScanningFilesystemSharedCache::getFileContents( +StringRef Filename, llvm::sys::fs::UniqueID UID) { + CacheShard &Sha

[PATCH] D114966: [clang][deps] Split stat and file content caches

dexonsmith added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:200-210 + // FIXME: This read can fail. + // In that case, we should probably update `CacheEntry::MaybeStat`. + // However, that is conc

[PATCH] D114966: [clang][deps] Split stat and file content caches

dexonsmith added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:25-31 llvm::ErrorOr Stat = (*MaybeFile)->status(); if (!Stat) return Stat.getError(); llvm::vfs::File &F = **MaybeFile; llvm::ErrorOr> MaybeBuff