[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:117
+    std::mutex CacheLock;
+    llvm::StringMap<std::unique_ptr<SharedFileSystemEntry>> Cache;
+  };
----------------
aganea wrote:
> Why not use a bump allocator here? (it would be per-thread) On Windows the heap is always thread-safe, which induce a lock in `malloc` for each new entry. You could also avoid the usage of `unique_ptr` by the same occasion:
> 
> `llvm::StringMap<SharedFileSystemEntry, SpecificBumpPtrAllocator<SharedFileSystemEntry>> Cache;`
> 
> //(unless you're planning on removing entries from the cache later on?)//
Good idea, I switched to a bump pointer allocator (I don't think I can use a specific one, as that would cause the values to be destroyed twice). I'm not planning on removing entries from the cache, no.


================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:76
+CachedFileSystemEntry
+CachedFileSystemEntry::createDirectoryEntry(llvm::vfs::Status Stat) {
+  assert(Stat.isDirectory() && "not a directory!");
----------------
aganea wrote:
> `llvm::vfs::Status &&Stat` to avoid a copy?
The copy should already be avoided, as I move the argument when passing in, and when it's assigned to the result.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63907/new/

https://reviews.llvm.org/D63907