[llvm][support] Implement tracing virtual file system #88326
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
jansvoboda11
force-pushed
the
vfs-tracing
branch
from
540321e to
b395e90
Compare
jansvoboda11
changed the title
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) ChangesLLVM-based tools often use the Full diff: https://github.com/llvm/llvm-project/pull/88326.diff 4 Files Affected:
diff --git a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index 697b7d70ff035a..c1a06c1f5ea20e 100644
--- a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++ b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -13,33 +13,11 @@
using namespace clang::tooling::dependencies;
-namespace {
-struct InstrumentingFilesystem
- : llvm::RTTIExtends<InstrumentingFilesystem, llvm::vfs::ProxyFileSystem> {
- unsigned NumStatusCalls = 0;
- unsigned NumGetRealPathCalls = 0;
-
- using llvm::RTTIExtends<InstrumentingFilesystem,
- llvm::vfs::ProxyFileSystem>::RTTIExtends;
-
- llvm::ErrorOr<llvm::vfs::Status> status(const llvm::Twine &Path) override {
- ++NumStatusCalls;
- return ProxyFileSystem::status(Path);
- }
-
- std::error_code getRealPath(const llvm::Twine &Path,
- llvm::SmallVectorImpl<char> &Output) override {
- ++NumGetRealPathCalls;
- return ProxyFileSystem::getRealPath(Path, Output);
- }
-};
-} // namespace
-
TEST(DependencyScanningWorkerFilesystem, CacheStatusFailures) {
auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
auto InstrumentingFS =
- llvm::makeIntrusiveRefCnt<InstrumentingFilesystem>(InMemoryFS);
+ llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
DependencyScanningFilesystemSharedCache SharedCache;
DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
@@ -65,7 +43,7 @@ TEST(DependencyScanningFilesystem, CacheGetRealPath) {
InMemoryFS->addFile("/bar", 0, llvm::MemoryBuffer::getMemBuffer(""));
auto InstrumentingFS =
- llvm::makeIntrusiveRefCnt<InstrumentingFilesystem>(InMemoryFS);
+ llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
DependencyScanningFilesystemSharedCache SharedCache;
DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h
index 4b1ca0c3d262b6..1d851d4d9aea51 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -1125,6 +1125,54 @@ class YAMLVFSWriter {
void write(llvm::raw_ostream &OS);
};
+/// File system that tracks the number of calls to the underlying file system.
+/// This is particularly useful when wrapped around \c RealFileSystem to add
+/// lightweight tracking of expensive syscalls.
+class TracingFileSystem
+ : public llvm::RTTIExtends<TracingFileSystem, ProxyFileSystem> {
+public:
+ static const char ID;
+
+ std::size_t NumStatusCalls = 0;
+ std::size_t NumOpenFileForReadCalls = 0;
+ std::size_t NumDirBeginCalls = 0;
+ std::size_t NumGetRealPathCalls = 0;
+ std::size_t NumIsLocalCalls = 0;
+
+ TracingFileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
+ : RTTIExtends(std::move(FS)) {}
+
+ ErrorOr<Status> status(const Twine &Path) override {
+ ++NumStatusCalls;
+ return ProxyFileSystem::status(Path);
+ }
+
+ ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override {
+ ++NumOpenFileForReadCalls;
+ return ProxyFileSystem::openFileForRead(Path);
+ }
+
+ directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override {
+ ++NumDirBeginCalls;
+ return ProxyFileSystem::dir_begin(Dir, EC);
+ }
+
+ std::error_code getRealPath(const Twine &Path,
+ SmallVectorImpl<char> &Output) override {
+ ++NumGetRealPathCalls;
+ return ProxyFileSystem::getRealPath(Path, Output);
+ }
+
+ std::error_code isLocal(const Twine &Path, bool &Result) override {
+ ++NumIsLocalCalls;
+ return ProxyFileSystem::isLocal(Path, Result);
+ }
+
+protected:
+ void printImpl(raw_ostream &OS, PrintType Type,
+ unsigned IndentLevel) const override;
+};
+
} // namespace vfs
} // namespace llvm
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index 32b480028e71b4..fe8924585e1ceb 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -2877,8 +2877,21 @@ recursive_directory_iterator::increment(std::error_code &EC) {
return *this;
}
+void TracingFileSystem::printImpl(raw_ostream &OS, PrintType Type,
+ unsigned IndentLevel) const {
+ printIndent(OS, IndentLevel);
+ OS << "TracingFileSystem\n";
+ if (Type == PrintType::Summary)
+ return;
+
+ if (Type == PrintType::Contents)
+ Type = PrintType::Summary;
+ getUnderlyingFS().print(OS, Type, IndentLevel + 1);
+}
+
const char FileSystem::ID = 0;
const char OverlayFileSystem::ID = 0;
const char ProxyFileSystem::ID = 0;
const char InMemoryFileSystem::ID = 0;
const char RedirectingFileSystem::ID = 0;
+const char TracingFileSystem::ID = 0;
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index 49a2e19e4f74cd..c692590eca2725 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -3395,3 +3395,53 @@ TEST(RedirectingFileSystemTest, ExternalPaths) {
EXPECT_EQ(CheckFS->SeenPaths, Expected);
}
+
+TEST(TracingFileSystemTest, TracingWorks) {
+ auto InMemoryFS = makeIntrusiveRefCnt<vfs::InMemoryFileSystem>();
+ auto TracingFS =
+ makeIntrusiveRefCnt<vfs::TracingFileSystem>(std::move(InMemoryFS));
+
+ EXPECT_EQ(TracingFS->NumStatusCalls, 0u);
+ EXPECT_EQ(TracingFS->NumOpenFileForReadCalls, 0u);
+ EXPECT_EQ(TracingFS->NumDirBeginCalls, 0u);
+ EXPECT_EQ(TracingFS->NumGetRealPathCalls, 0u);
+ EXPECT_EQ(TracingFS->NumIsLocalCalls, 0u);
+
+ (void)TracingFS->status("/foo");
+ EXPECT_EQ(TracingFS->NumStatusCalls, 1u);
+ EXPECT_EQ(TracingFS->NumOpenFileForReadCalls, 0u);
+ EXPECT_EQ(TracingFS->NumDirBeginCalls, 0u);
+ EXPECT_EQ(TracingFS->NumGetRealPathCalls, 0u);
+ EXPECT_EQ(TracingFS->NumIsLocalCalls, 0u);
+
+ (void)TracingFS->openFileForRead("/foo");
+ EXPECT_EQ(TracingFS->NumStatusCalls, 1u);
+ EXPECT_EQ(TracingFS->NumOpenFileForReadCalls, 1u);
+ EXPECT_EQ(TracingFS->NumDirBeginCalls, 0u);
+ EXPECT_EQ(TracingFS->NumGetRealPathCalls, 0u);
+ EXPECT_EQ(TracingFS->NumIsLocalCalls, 0u);
+
+ std::error_code EC;
+ (void)TracingFS->dir_begin("/foo", EC);
+ EXPECT_EQ(TracingFS->NumStatusCalls, 1u);
+ EXPECT_EQ(TracingFS->NumOpenFileForReadCalls, 1u);
+ EXPECT_EQ(TracingFS->NumDirBeginCalls, 1u);
+ EXPECT_EQ(TracingFS->NumGetRealPathCalls, 0u);
+ EXPECT_EQ(TracingFS->NumIsLocalCalls, 0u);
+
+ SmallString<128> RealPath;
+ (void)TracingFS->getRealPath("/foo", RealPath);
+ EXPECT_EQ(TracingFS->NumStatusCalls, 1u);
+ EXPECT_EQ(TracingFS->NumOpenFileForReadCalls, 1u);
+ EXPECT_EQ(TracingFS->NumDirBeginCalls, 1u);
+ EXPECT_EQ(TracingFS->NumGetRealPathCalls, 1u);
+ EXPECT_EQ(TracingFS->NumIsLocalCalls, 0u);
+
+ bool IsLocal;
+ (void)TracingFS->isLocal("/foo", IsLocal);
+ EXPECT_EQ(TracingFS->NumStatusCalls, 1u);
+ EXPECT_EQ(TracingFS->NumOpenFileForReadCalls, 1u);
+ EXPECT_EQ(TracingFS->NumDirBeginCalls, 1u);
+ EXPECT_EQ(TracingFS->NumGetRealPathCalls, 1u);
+ EXPECT_EQ(TracingFS->NumIsLocalCalls, 1u);
+}
|
jansvoboda11
commented
Apr 12, 2024
| public: | ||
| static const char ID; | ||
|
|
||
| std::size_t NumStatusCalls = 0; |
There was a problem hiding this comment.
I'm contemplating whether to make these atomic. The root FileSystem class inherits from ThreadSafeRefCountedBase, suggesting that VFSs may be thread-safe.
For example in clang-scan-deps, all workers could use single RealFileSystem. If we wrapped that in a single thread-safe TracingFileSystem, we'd very easily get the complete VFS stats. The current non-thread-safe approach would require having N of these VFS stacks and then manually aggregating the individual VFS stats.
Thoughts?
There was a problem hiding this comment.
Looking at the existing VFSs there appears to be a reasonable subset that currently are thread safe for stat/open/etc., but some definitely aren't. I think it's fine to make this one thread safe and document that it is if the underlying FS is.
I think they can also all be relaxed memory order, you have to have some other synchronization before collecting the numbers at the end anyway.
There was a problem hiding this comment.
There was a problem hiding this comment.
I'm going to land this with non-atomic internals and add that feature when/if the need arises.
jansvoboda11
force-pushed
the
vfs-tracing
branch
3 times, most recently
from
44791e8 to
0afe6a5
Compare
Bigcheese
approved these changes
Apr 17, 2024
| public: | ||
| static const char ID; | ||
|
|
||
| std::size_t NumStatusCalls = 0; |
There was a problem hiding this comment.
Looking at the existing VFSs there appears to be a reasonable subset that currently are thread safe for stat/open/etc., but some definitely aren't. I think it's fine to make this one thread safe and document that it is if the underlying FS is.
I think they can also all be relaxed memory order, you have to have some other synchronization before collecting the numbers at the end anyway.
jansvoboda11
force-pushed
the
vfs-tracing
branch
from
0afe6a5 to
af44789
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/2/builds/6155 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/6432 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/4523 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/8147 Here is the relevant piece of the build log for the reference |
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Nov 4, 2024
LLVM-based tools often use the `llvm::vfs::FileSystem` instrastructure to access the file system. This patch adds new kind of a VFS that performs lightweight tracing of file system operations on an underlying VFS. This is supposed to aid in investigating file system traffic without resorting to instrumentation on the operating system level. There will be follow-up patches that integrate this into Clang and its dependency scanner. (cherry picked from commit 70fcdb3)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LLVM-based tools often use the
llvm::vfs::FileSysteminstrastructure to access the file system. This patch adds new kind of a VFS that performs lightweight tracing of file system operations on an underlying VFS. This is supposed to aid in investigating file system traffic without resorting to instrumentation on the operating system level. There will be follow-up patches that integrate this into Clang and its dependency scanner.