Skip to content

[llvm][support] Implement tracing virtual file system #88326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,11 @@

using namespace clang::tooling::dependencies;

namespace {
struct InstrumentingFilesystem
: llvm::RTTIExtends<InstrumentingFilesystem, llvm::vfs::ProxyFileSystem> {
unsigned NumStatusCalls = 0;
unsigned NumGetRealPathCalls = 0;
unsigned NumExistsCalls = 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);
}

bool exists(const llvm::Twine &Path) override {
++NumExistsCalls;
return ProxyFileSystem::exists(Path);
}
};
} // 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);
Expand All @@ -71,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);
Expand Down Expand Up @@ -157,7 +129,7 @@ TEST(DependencyScanningFilesystem, RealPathAndStatusInvariants) {
TEST(DependencyScanningFilesystem, CacheStatOnExists) {
auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
auto InstrumentingFS =
llvm::makeIntrusiveRefCnt<InstrumentingFilesystem>(InMemoryFS);
llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
InMemoryFS->setCurrentWorkingDirectory("/");
InMemoryFS->addFile("/foo", 0, llvm::MemoryBuffer::getMemBuffer(""));
InMemoryFS->addFile("/bar", 0, llvm::MemoryBuffer::getMemBuffer(""));
Expand Down
54 changes: 54 additions & 0 deletions llvm/include/llvm/Support/VirtualFileSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,60 @@ 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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@jansvoboda11 jansvoboda11 Aug 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aganea Any thoughts? I'm asking because of #88427. I think that making the counters atomic in this PR is not that problematic, since you can still choose how you use the VFS (i.e. how many per core you share), unlike the originally global counters in FileManager in your PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to land this with non-atomic internals and add that feature when/if the need arises.

std::size_t NumOpenFileForReadCalls = 0;
std::size_t NumDirBeginCalls = 0;
std::size_t NumGetRealPathCalls = 0;
std::size_t NumExistsCalls = 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);
}

bool exists(const Twine &Path) override {
++NumExistsCalls;
return ProxyFileSystem::exists(Path);
}

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

Expand Down
26 changes: 26 additions & 0 deletions llvm/lib/Support/VirtualFileSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2945,8 +2945,34 @@ 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;

printIndent(OS, IndentLevel);
OS << "NumStatusCalls=" << NumStatusCalls << "\n";
printIndent(OS, IndentLevel);
OS << "NumOpenFileForReadCalls=" << NumOpenFileForReadCalls << "\n";
printIndent(OS, IndentLevel);
OS << "NumDirBeginCalls=" << NumDirBeginCalls << "\n";
printIndent(OS, IndentLevel);
OS << "NumGetRealPathCalls=" << NumGetRealPathCalls << "\n";
printIndent(OS, IndentLevel);
OS << "NumExistsCalls=" << NumExistsCalls << "\n";
printIndent(OS, IndentLevel);
OS << "NumIsLocalCalls=" << NumIsLocalCalls << "\n";

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;
114 changes: 114 additions & 0 deletions llvm/unittests/Support/VirtualFileSystemTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3565,3 +3565,117 @@ TEST(RedirectingFileSystemTest, ExistsRedirectOnly) {
EXPECT_FALSE(Redirecting->exists("/b"));
EXPECT_TRUE(Redirecting->exists("/vfile"));
}

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->NumExistsCalls, 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->NumExistsCalls, 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->NumExistsCalls, 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->NumExistsCalls, 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->NumExistsCalls, 0u);
EXPECT_EQ(TracingFS->NumIsLocalCalls, 0u);

(void)TracingFS->exists("/foo");
EXPECT_EQ(TracingFS->NumStatusCalls, 1u);
EXPECT_EQ(TracingFS->NumOpenFileForReadCalls, 1u);
EXPECT_EQ(TracingFS->NumDirBeginCalls, 1u);
EXPECT_EQ(TracingFS->NumGetRealPathCalls, 1u);
EXPECT_EQ(TracingFS->NumExistsCalls, 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->NumExistsCalls, 1u);
EXPECT_EQ(TracingFS->NumIsLocalCalls, 1u);
}

TEST(TracingFileSystemTest, PrintOutput) {
auto InMemoryFS = makeIntrusiveRefCnt<vfs::InMemoryFileSystem>();
auto TracingFS =
makeIntrusiveRefCnt<vfs::TracingFileSystem>(std::move(InMemoryFS));

(void)TracingFS->status("/foo");

(void)TracingFS->openFileForRead("/foo");
(void)TracingFS->openFileForRead("/foo");

std::error_code EC;
(void)TracingFS->dir_begin("/foo", EC);
(void)TracingFS->dir_begin("/foo", EC);
(void)TracingFS->dir_begin("/foo", EC);

llvm::SmallString<128> RealPath;
(void)TracingFS->getRealPath("/foo", RealPath);
(void)TracingFS->getRealPath("/foo", RealPath);
(void)TracingFS->getRealPath("/foo", RealPath);
(void)TracingFS->getRealPath("/foo", RealPath);

(void)TracingFS->exists("/foo");
(void)TracingFS->exists("/foo");
(void)TracingFS->exists("/foo");
(void)TracingFS->exists("/foo");
(void)TracingFS->exists("/foo");

bool IsLocal;
(void)TracingFS->isLocal("/foo", IsLocal);
(void)TracingFS->isLocal("/foo", IsLocal);
(void)TracingFS->isLocal("/foo", IsLocal);
(void)TracingFS->isLocal("/foo", IsLocal);
(void)TracingFS->isLocal("/foo", IsLocal);
(void)TracingFS->isLocal("/foo", IsLocal);

std::string Output;
llvm::raw_string_ostream OS(Output);
TracingFS->print(OS);

ASSERT_EQ("TracingFileSystem\n"
"NumStatusCalls=1\n"
"NumOpenFileForReadCalls=2\n"
"NumDirBeginCalls=3\n"
"NumGetRealPathCalls=4\n"
"NumExistsCalls=5\n"
"NumIsLocalCalls=6\n"
" InMemoryFileSystem\n",
Output);
}
Loading