[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss added a comment.

In D136651#4067012 , @glandium wrote:

> Oh yes, we're using `-DLLVM_LINK_LLVM_DYLIB=ON` too, so that would be the 
> main trigger.

This was indeed the main trigger. Thanks for the help narrowing this down. I 
reverted for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss added a comment.

In D136651#4066960 , @mstorsjo wrote:

> In D136651#4066949 , @friss wrote:
>
>> Thanks for the report, unfortunately I have no way of testing this setup.
>
> Can you try a build with `-DLLVM_LINK_LLVM_DYLIB=ON`?

Trying. If that's indeed the issue (and not the cross-compile environment), 
I'll revert while looking at it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss added a comment.

In D136651#4064474 , @glandium wrote:

> In D136651#4064260 , @glandium 
> wrote:
>
>> This broke our mac builds with errors like:
>>
>>   ld64.lld: error: undefined symbol: CFRunLoopRun
>>   >>> referenced by 
>> tools/clang/tools/clang-stat-cache/CMakeFiles/clang-stat-cache.dir/clang-stat-cache.cpp.o:(symbol
>>  main+0x11be)
>>
>> (and many more symbols)
>
> Additional information: those are cross-compiled, and for some reason the 
> `-framework CoreServices` in the CMakeLists.txt doesn't seem to make it to 
> the command line, which is surprising because something similar works fine 
> for e.g. dsymutil.

Thanks for the report, unfortunately I have no way of testing this setup. Any 
chance you can experiment on your side? DirectoryWatcher uses the came decency, 
I'm wondering why it's not an issue there. If it helps to revert the change 
while someone investigates, that's perfectly fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss added a comment.

In D136651#4063632 , @lebedev.ri 
wrote:

> In D136651#4063597 , @friss wrote:
>
>> In D136651#4060071 , @lebedev.ri 
>> wrote:
>>
>>> Docs still missing.
>>
>> Sorry @lebedev.ri I missed your earlier comment about this. What format of 
>> doc would you like to see? A more elaborate comment in the tool's source or 
>> something else?
>
> Right now it's impossible to discover that tool unless you already know it 
> exists (or it's mentioned in some xcode doc, i guess)
> It should have it's own description in it's `--help`, and ideally an `.rst` 
> documentation page in `docs`.

I have no issue adding a more detailed help text. The documentation page seems 
somewhat overkill, but I guess I could be convinced otherwise. You need a 
fairly special build environment for this to make a difference and I'm not sure 
it's broadly applicable. You also need good build system integration to make it 
practical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss added a subscriber: lebedev.ri.
friss added a comment.

In D136651#4060071 , @lebedev.ri 
wrote:

> Docs still missing.

Sorry @lebedev.ri I missed your earlier comment about this. What format of doc 
would you like to see? A more elaborate comment in the tool's source or 
something else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa033dbbe5c43: [Clang] Give Clang the ability to use a shared 
stat cache (authored by friss).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  llvm/include/llvm/Support/StatCacheFileSystem.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/StatCacheFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -14,9 +14,11 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/StatCacheFileSystem.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3228,3 +3230,306 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+sys::fs::file_status s;
+status(BaseDir, s);
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, s, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+// Base path should be present in the stat cache.
+Filenames.push_back(std::string(BaseDir));
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+VersionMatch, FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache =
+vfs::StatCacheFileSystem::create(std::move(*ErrorOrBuffer), Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 489968.
friss added a comment.

More Windows fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  llvm/include/llvm/Support/StatCacheFileSystem.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/StatCacheFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -14,9 +14,11 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/StatCacheFileSystem.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3228,3 +3230,306 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+sys::fs::file_status s;
+status(BaseDir, s);
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, s, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+// Base path should be present in the stat cache.
+Filenames.push_back(std::string(BaseDir));
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+VersionMatch, FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache =
+vfs::StatCacheFileSystem::create(std::move(*ErrorOrBuffer), Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy contents");
+  TempFile _ace(TestDirectory.path("a/c/e"));
+  

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 489914.
friss added a comment.

The pre-commit CI showed some test failures on Windows. Try to address these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  llvm/include/llvm/Support/StatCacheFileSystem.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/StatCacheFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -14,9 +14,11 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/StatCacheFileSystem.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3228,3 +3230,306 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+sys::fs::file_status s;
+status(BaseDir, s);
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, s, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+// Base path should be present in the stat cache.
+Filenames.push_back(std::string(BaseDir));
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+VersionMatch, FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache =
+vfs::StatCacheFileSystem::create(std::move(*ErrorOrBuffer), Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy 

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss added a comment.

In D136651#4030008 , @benlangmuir 
wrote:

> Re-added a few of my minor comments that I think got lost on a specific 
> version of the diff. Otherwise LGTM.

Sorry for missing these. I think I addressed all of them now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 486711.
friss added a comment.

Fix Windows build (Thanks @ravi-ramaseshan)
Address @benlangmuir comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  llvm/include/llvm/Support/StatCacheFileSystem.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/StatCacheFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -14,9 +14,11 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/StatCacheFileSystem.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3228,3 +3230,306 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+sys::fs::file_status s;
+status(BaseDir, s);
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, s, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+// Base path should be present in the stat cache.
+Filenames.push_back(std::string(BaseDir));
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+VersionMatch, FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache =
+vfs::StatCacheFileSystem::create(std::move(*ErrorOrBuffer), Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy 

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 486466.
friss added a comment.

Rebase correctly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  llvm/include/llvm/Support/StatCacheFileSystem.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/StatCacheFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -14,9 +14,11 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/StatCacheFileSystem.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3228,3 +3230,306 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+sys::fs::file_status s;
+status(BaseDir, s);
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, s, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+// Base path should be present in the stat cache.
+Filenames.push_back(std::string(BaseDir));
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+VersionMatch, FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache = vfs::StatCacheFileSystem::create(
+std::move(*ErrorOrBuffer), OutputFile, Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy contents");
+  TempFile 

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss added inline comments.



Comment at: clang/tools/clang-stat-cache/clang-stat-cache.cpp:291
+  IsCaseSensitive =
+  ::pathconf(TargetDirectory.c_str(), _PC_CASE_SENSITIVE) == 1;
+#endif

benlangmuir wrote:
> Is this pathconf extension Darwin-only?
I don't think so. It's a BSD extension.



Comment at: clang/tools/clang-stat-cache/clang-stat-cache.cpp:300
+  auto endTime = llvm::TimeRecord::getCurrentTime();
+  endTime -= startTime;
+

benlangmuir wrote:
> Nit: move subtraction to the print or create a new "duration" variable 
> instead of mutating endTime. Same below for writeStatCache.
I would have done so originally if there was a TimeRecord overload that allows 
simple subtraction. I don't think that exists. I renamed the variable to 
`duration`, maybe it make the code a little more readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 486439.
friss added a comment.

Address review feedback
Move StatCacheFileSystem to its own file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  clang/tools/driver/features.json
  llvm/include/llvm/Support/StatCacheFileSystem.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/StatCacheFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -14,9 +14,11 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/StatCacheFileSystem.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3388,3 +3390,306 @@
   EXPECT_FALSE(Redirecting->exists("/b"));
   EXPECT_TRUE(Redirecting->exists("/vfile"));
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+sys::fs::file_status s;
+status(BaseDir, s);
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, s, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+// Base path should be present in the stat cache.
+Filenames.push_back(std::string(BaseDir));
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+VersionMatch, FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache = vfs::StatCacheFileSystem::create(
+std::move(*ErrorOrBuffer), OutputFile, Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile 

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 472379.
friss added a comment.

Fix unit test build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  clang/tools/driver/features.json
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3159,3 +3160,302 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  sys::fs::file_status s;
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+VersionMatch, FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache = vfs::StatCacheFileSystem::create(
+std::move(*ErrorOrBuffer), OutputFile, Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy contents");
+  TempFile _ace(TestDirectory.path("a/c/e"));
+  TempFile _acf(TestDirectory.path("a/c/f"), "", "More dummy contents");
+  TempDir _ag(TestDirectory.path("a/g"));
+  TempFile _agh(TestDirectory.path("a/g/h"));
+
+  StringRef BaseDir(_a.path());
+
+  SmallVector Filenames;
+  IntrusiveRefCntPtr StatCacheFS;
+  createStatCacheFileSystem(TestDirectory.path("stat.cache"), BaseDir,
+/* 

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 472355.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  clang/tools/driver/features.json
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3159,3 +3160,302 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  sys::fs::file_status s;
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache = vfs::StatCacheFileSystem::create(
+std::move(*ErrorOrBuffer), OutputFile, Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy contents");
+  TempFile _ace(TestDirectory.path("a/c/e"));
+  TempFile _acf(TestDirectory.path("a/c/f"), "", "More dummy contents");
+  TempDir _ag(TestDirectory.path("a/g"));
+  TempFile _agh(TestDirectory.path("a/g/h"));
+
+  StringRef BaseDir(_a.path());
+
+  SmallVector Filenames;
+  IntrusiveRefCntPtr StatCacheFS;
+  createStatCacheFileSystem(TestDirectory.path("stat.cache"), BaseDir,
+/* IsCaseSensitive= */ true, StatCacheFS,
+ 

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 472321.
friss added a comment.

Add version number in cache file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3159,3 +3160,302 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  sys::fs::file_status s;
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache = vfs::StatCacheFileSystem::create(
+std::move(*ErrorOrBuffer), OutputFile, Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy contents");
+  TempFile _ace(TestDirectory.path("a/c/e"));
+  TempFile _acf(TestDirectory.path("a/c/f"), "", "More dummy contents");
+  TempDir _ag(TestDirectory.path("a/g"));
+  TempFile _agh(TestDirectory.path("a/g/h"));
+
+  StringRef BaseDir(_a.path());
+
+  SmallVector Filenames;
+  IntrusiveRefCntPtr StatCacheFS;
+  createStatCacheFileSystem(TestDirectory.path("stat.cache"), BaseDir,
+/* IsCaseSensitive= */ true, 

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss added inline comments.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:3002
+// The format of the stat cache is (pseudo-code):
+//  struct stat_cache {
+//char Magic[4];   // "STAT" or "Stat"

steven_wu wrote:
> Few comments about stats representation.
> 1. Can we version the stat cache file so we can evolve it in the future if 
> needed?
> 2. I wonder if we need to have a more flexible representation for DataType 
> other than `sys::fs::file_status`. Current entry is locked with the endian of 
> the host and can't be used to encode more information than file_status. 
> 1. Can we version the stat cache file so we can evolve it in the future if 
> needed?

Yes, we can. I pondered doing it in the original patch but didn't see a reason 
this would evolve in the future. Of course, I can't predict the future, so 
maybe a version field is the safe approach.

> 2. I wonder if we need to have a more flexible representation for DataType 
> other than `sys::fs::file_status`. Current entry is locked with the endian of 
> the host and can't be used to encode more information than file_status. 

The non-portability of the current cache doesn't bother me as it's meant to 
mirror filesystem aspects that are local to a machine. Doesn't adding a version 
number alleviate some of these concerns? If we wanted to add something else in 
the future, or make it portable, we could just bump that.

I think your comment hints at a higher-level question though. If we were to add 
something else to the cached data, it wouldn't be a stat cache anymore. Should 
the feature be a more general "filesystem cache" which starts out with just 
stat data? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 470644.
friss added a comment.

Address some review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3159,3 +3160,300 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  sys::fs::file_status s;
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache = vfs::StatCacheFileSystem::create(
+std::move(*ErrorOrBuffer), OutputFile, Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy contents");
+  TempFile _ace(TestDirectory.path("a/c/e"));
+  TempFile _acf(TestDirectory.path("a/c/f"), "", "More dummy contents");
+  TempDir _ag(TestDirectory.path("a/g"));
+  TempFile _agh(TestDirectory.path("a/g/h"));
+
+  StringRef BaseDir(_a.path());
+
+  SmallVector Filenames;
+  IntrusiveRefCntPtr StatCacheFS;
+  createStatCacheFileSystem(TestDirectory.path("stat.cache"), BaseDir,
+/* IsCaseSensitive= */ true, StatCacheFS,
+Filenames);
+  

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss added a comment.

Thanks for the initial feedback!

> Mostly just skimmed so far, will hopefully have some time to look at this 
> more tomorrow. Out of interest, do you have any performance numbers using 
> this change? I assume this mainly impacts implicit modules (since I suspect 
> we'd also be opening the file as well anyway), is that true?

You're correct that this overhead has been measured on implicit module builds. 
As I mentioned in the commit message this saves over 20% of the overall built 
time in some cases. This time is split between module validation (which could 
be skipped) and HeaderSearch (which cannot be skipped).




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:5115-5130
+  for (const auto  : CI.getHeaderSearchOpts().VFSStatCacheFiles) {
+llvm::ErrorOr> Buffer =
+llvm::MemoryBuffer::getFile(File);
+if (!Buffer) {
+  Diags.Report(diag::err_missing_vfs_stat_cache_file) << File;
+  continue;
+}

bnbarham wrote:
> IMO VFS overlays being modeled like this is a mistake and makes 
> reasoning/testing about them fairly difficult. I have a PR up 
> https://reviews.llvm.org/D121423 to change `OverlayFileSystem` to make more 
> sense and be a real overlay rather than what it is now. If I finish that one 
> off, how would you feel about changing the behavior of `StatCacheFileSystem` 
> to just immediately error if it doesn't contain the file, rather than proxy 
> (and thus chain) as it does now?
> 
> So for multiple files we'd then have:
>   - OverlayFileSystem
> - StatCacheFileSystem
> - StatCacheFileSystem
> - RealFileSystem
> 
> Then any non-stat or exists call would return 
> `llvm::errc::no_such_file_or_directory` and then the next FS would be used 
> instead.
> 
> I don't think this *really* matters for `StatCacheFileSystem`, so I'm fine if 
> you'd rather not wait for me to change `OverlayFileSystem`. I can make the 
> changes myself after getting my PR in.
I don't think that's really doable if you want to keep the ability to cache 
negative hits. If a miss always results in a query to the real filesystem, then 
you're not saving the `stat` call. A lot of the time is spent in HeaderSearch 
which queries a similar number of non-existing and existing files.
But I'm not dead set on this. I also haven't spent a lot of time thinking about 
your proposal.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:2959-2960
+// the pathis different. The canonicalization that the call to 
remove_dots()
+// does leaves only '..' with symlinks as a source of confusion. If the 
path
+// does not contain '..' we can safely say it doesn't exist.
+if (std::find(sys::path::begin(SuffixPath), sys::path::end(SuffixPath),

bnbarham wrote:
> This sentence is a little confusing to me. `remove_dots` just leaves `..` 
> unless you pass `remove_dot_dot` (but it doesn't do any checking). IMO just 
> the `If the path does not contain '..' we can safely say it doesn't exist.` 
> is enough.
`remove_dots` does more than remove dots. It is confusing, but it also removes 
excess separators (see the `Canonical` unit test). This means that the cache 
will work for /path/to/cache/file/a as well as /path/to/cache/file///a and 
/path/to/cache/file/././a. There are basically infinite spellings of a path 
just by adding `.` and additional separators.
`..` is interesting because it's not semantically preserving to remove them in 
the presence of symlinks.
I'm fine with simplifying the description, but that is the nuance I tried to 
convey.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:2961
+// does not contain '..' we can safely say it doesn't exist.
+if (std::find(sys::path::begin(SuffixPath), sys::path::end(SuffixPath),
+  "..") == sys::path::end(SuffixPath)) {

bnbarham wrote:
> FWIW `StringRef` has a `contains`
But that wouldn't be correct. Here we are looking for a path component which is 
`..`. A simple text search would fire on a filename containing `..`. I think 
this search on components is the only correct way to do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss created this revision.
friss added reviewers: jansvoboda11, bnbarham, arphaman.
Herald added a subscriber: hiraditya.
Herald added a project: All.
friss requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

  Every Clang instance uses an internal FileSystemStatCache to avoid
  stating the same content multiple times. However, different instances
  of Clang will contend for filesystem access for their initial stats
  during HeaderSearch or module validation.
  
  On some workloads, the time spent in the kernel in these concurrent
  stat calls has been measured to be over 20% of the overall compilation
  time. This is extremly wassteful when most of the stat calls target
  mostly immutable content like a SDK.
  
  This commit introduces a new tool `clang-stat-cache` able to generate
  an OnDiskHashmap containing the stat data for a given filesystem
  hierarchy.
  
  The driver part of this has been modeled after -ivfsoverlay given
  the similarities with what it influences. It introduces a new
  -ivfsstatcache driver option to instruct Clang to use a stat cache
  generated by `clang-stat-cache`. These stat caches are inserted at
  the bottom of the VFS stack (right above the real filesystem).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3159,3 +3160,300 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr ,
+  StringCollection ,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  sys::fs::file_status s;
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr ) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache = vfs::StatCacheFileSystem::create(
+std::move(*ErrorOrBuffer), OutputFile, Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection ) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto  : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+