sammccall created this revision.
sammccall added a reviewer: bkramer.
Herald added subscribers: cfe-commits, fedor.sergeev.
Most callers I can find are using only `getName()`. Type is used by the
recursive iterator.
Now we don't have to call stat() on every listed file (on most platforms).
Exceptions are e.g. Solaris where readdir() doesn't include type information.
On those platforms we'll still stat() - see https://reviews.llvm.org/D51918.
The result is significantly faster (stat() can be slow).
My motivation: this may allow us to improve clang IO on large TUs with long
include search paths. Caching readdir() results may allow us to skip many stat()
and open() operations on nonexistent files.
Repository:
rC Clang
https://reviews.llvm.org/D51921
Files:
include/clang/Basic/VirtualFileSystem.h
lib/Basic/VirtualFileSystem.cpp
unittests/Basic/VirtualFileSystemTest.cpp
Index: unittests/Basic/VirtualFileSystemTest.cpp
===================================================================
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -104,21 +104,23 @@
Path(_Path.str()) {
for ( ; I != FilesAndDirs.end(); ++I) {
if (isInPath(I->first)) {
- CurrentEntry = I->second;
+ CurrentEntry.Path = I->second.getName();
+ CurrentEntry.Type = I->second.getType();
break;
}
}
}
std::error_code increment() override {
++I;
for ( ; I != FilesAndDirs.end(); ++I) {
if (isInPath(I->first)) {
- CurrentEntry = I->second;
+ CurrentEntry.Path = I->second.getName();
+ CurrentEntry.Type = I->second.getType();
break;
}
}
if (I == FilesAndDirs.end())
- CurrentEntry = vfs::Status();
+ CurrentEntry = vfs::directory_entry();
return std::error_code();
}
};
@@ -656,14 +658,14 @@
O->pushOverlay(Upper);
std::error_code EC;
- Lower->addRegularFile("/onlyInLow", sys::fs::owner_read);
- Lower->addRegularFile("/hiddenByMid", sys::fs::owner_read);
- Lower->addRegularFile("/hiddenByUp", sys::fs::owner_read);
- Middle->addRegularFile("/onlyInMid", sys::fs::owner_write);
- Middle->addRegularFile("/hiddenByMid", sys::fs::owner_write);
- Middle->addRegularFile("/hiddenByUp", sys::fs::owner_write);
- Upper->addRegularFile("/onlyInUp", sys::fs::owner_all);
- Upper->addRegularFile("/hiddenByUp", sys::fs::owner_all);
+ Lower->addRegularFile("/onlyInLow");
+ Lower->addDirectory("/hiddenByMid");
+ Lower->addDirectory("/hiddenByUp");
+ Middle->addRegularFile("/onlyInMid");
+ Middle->addRegularFile("/hiddenByMid");
+ Middle->addDirectory("/hiddenByUp");
+ Upper->addRegularFile("/onlyInUp");
+ Upper->addRegularFile("/hiddenByUp");
checkContents(
O->dir_begin("/", EC),
{"/hiddenByUp", "/onlyInUp", "/hiddenByMid", "/onlyInMid", "/onlyInLow"});
@@ -676,16 +678,16 @@
if (I->getName() == "/hiddenByUp")
break;
ASSERT_NE(E, I);
- EXPECT_EQ(sys::fs::owner_all, I->getPermissions());
+ EXPECT_EQ(sys::fs::file_type::regular_file, I->Type);
}
{
std::error_code EC;
vfs::directory_iterator I = O->dir_begin("/", EC), E;
for ( ; !EC && I != E; I.increment(EC))
if (I->getName() == "/hiddenByMid")
break;
ASSERT_NE(E, I);
- EXPECT_EQ(sys::fs::owner_write, I->getPermissions());
+ EXPECT_EQ(sys::fs::file_type::regular_file, I->Type);
}
}
Index: lib/Basic/VirtualFileSystem.cpp
===================================================================
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -316,25 +316,19 @@
public:
RealFSDirIter(const Twine &Path, std::error_code &EC) : Iter(Path, EC) {
if (Iter != llvm::sys::fs::directory_iterator()) {
- llvm::sys::fs::file_status S;
- std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
- CurrentEntry = Status::copyWithNewName(S, Iter->path());
- if (!EC)
- EC = ErrorCode;
+ CurrentEntry.Path = Iter->path();
+ CurrentEntry.Type = Iter->type();
}
}
std::error_code increment() override {
std::error_code EC;
Iter.increment(EC);
if (Iter == llvm::sys::fs::directory_iterator()) {
- CurrentEntry = Status();
+ CurrentEntry = directory_entry();
} else {
- llvm::sys::fs::file_status S;
- std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
- CurrentEntry = Status::copyWithNewName(S, Iter->path());
- if (!EC)
- EC = ErrorCode;
+ CurrentEntry.Path = Iter->path();
+ CurrentEntry.Type = Iter->type();
}
return EC;
}
@@ -446,11 +440,11 @@
while (true) {
std::error_code EC = incrementDirIter(IsFirstTime);
if (EC || CurrentDirIter == directory_iterator()) {
- CurrentEntry = Status();
+ CurrentEntry = directory_entry();
return EC;
}
CurrentEntry = *CurrentDirIter;
- StringRef Name = llvm::sys::path::filename(CurrentEntry.getName());
+ StringRef Name = llvm::sys::path::filename(CurrentEntry.Path);
if (SeenNames.insert(Name).second)
return EC; // name not seen before
}
@@ -850,11 +844,20 @@
if (I != E) {
SmallString<256> Path(RequestedDirName);
llvm::sys::path::append(Path, I->second->getFileName());
- CurrentEntry = detail::getNodeStatus(I->second.get(), Path);
+ CurrentEntry.Path = Path.str();
+ switch (I->second->getKind()) {
+ case detail::IME_File:
+ case detail::IME_HardLink:
+ CurrentEntry.Type = sys::fs::file_type::regular_file;
+ break;
+ case detail::IME_Directory:
+ CurrentEntry.Type = sys::fs::file_type::directory_file;
+ break;
+ }
} else {
// When we're at the end, make CurrentEntry invalid and DirIterImpl will
// do the rest.
- CurrentEntry = Status();
+ CurrentEntry = directory_entry();
}
}
@@ -1010,17 +1013,14 @@
static bool classof(const Entry *E) { return E->getKind() == EK_File; }
};
-class RedirectingFileSystem;
-
class VFSFromYamlDirIterImpl : public clang::vfs::detail::DirIterImpl {
std::string Dir;
- RedirectingFileSystem &FS;
RedirectingDirectoryEntry::iterator Current, End;
std::error_code incrementImpl();
public:
- VFSFromYamlDirIterImpl(const Twine &Path, RedirectingFileSystem &FS,
+ VFSFromYamlDirIterImpl(const Twine &Path,
RedirectingDirectoryEntry::iterator Begin,
RedirectingDirectoryEntry::iterator End,
std::error_code &EC);
@@ -1184,8 +1184,8 @@
}
auto *D = cast<RedirectingDirectoryEntry>(*E);
- return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(Dir,
- *this, D->contents_begin(), D->contents_end(), EC));
+ return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(
+ Dir, D->contents_begin(), D->contents_end(), EC));
}
void setExternalContentsPrefixDir(StringRef PrefixDir) {
@@ -2079,10 +2079,9 @@
}
VFSFromYamlDirIterImpl::VFSFromYamlDirIterImpl(
- const Twine &_Path, RedirectingFileSystem &FS,
- RedirectingDirectoryEntry::iterator Begin,
+ const Twine &_Path, RedirectingDirectoryEntry::iterator Begin,
RedirectingDirectoryEntry::iterator End, std::error_code &EC)
- : Dir(_Path.str()), FS(FS), Current(Begin), End(End) {
+ : Dir(_Path.str()), Current(Begin), End(End) {
EC = incrementImpl();
}
@@ -2096,23 +2095,20 @@
while (Current != End) {
SmallString<128> PathStr(Dir);
llvm::sys::path::append(PathStr, (*Current)->getName());
- llvm::ErrorOr<vfs::Status> S = FS.status(PathStr);
- if (!S) {
- // Skip entries which do not map to a reliable external content.
- if (FS.ignoreNonExistentContents() &&
- S.getError() == llvm::errc::no_such_file_or_directory) {
- ++Current;
- continue;
- } else {
- return S.getError();
- }
+ CurrentEntry.Path = PathStr.str();
+ switch ((*Current)->getKind()) {
+ case EK_Directory:
+ CurrentEntry.Type = sys::fs::file_type::directory_file;
+ break;
+ case EK_File:
+ CurrentEntry.Type = sys::fs::file_type::regular_file;
+ break;
}
- CurrentEntry = *S;
break;
}
if (Current == End)
- CurrentEntry = Status();
+ CurrentEntry = directory_entry();
return {};
}
@@ -2130,10 +2126,10 @@
vfs::recursive_directory_iterator &
recursive_directory_iterator::increment(std::error_code &EC) {
assert(FS && State && !State->empty() && "incrementing past end");
- assert(State->top()->isStatusKnown() && "non-canonical end iterator");
+ assert(!State->top()->Path.empty() && "non-canonical end iterator");
vfs::directory_iterator End;
- if (State->top()->isDirectory()) {
- vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
+ if (State->top()->Type == sys::fs::file_type::directory_file) {
+ vfs::directory_iterator I = FS->dir_begin(State->top()->Path, EC);
if (I != End) {
State->push(I);
return *this;
Index: include/clang/Basic/VirtualFileSystem.h
===================================================================
--- include/clang/Basic/VirtualFileSystem.h
+++ include/clang/Basic/VirtualFileSystem.h
@@ -126,18 +126,27 @@
virtual std::error_code close() = 0;
};
+/// A member of a directory, yielded by a directory_iterator.
+/// Only information available on most platforms is included.
+struct directory_entry {
+ std::string Path;
+ llvm::sys::fs::file_type Type;
+ // For compatibility with old Status-based API. Prefer using Path directly.
+ StringRef getName() const { return Path; }
+};
+
namespace detail {
/// An interface for virtual file systems to provide an iterator over the
/// (non-recursive) contents of a directory.
struct DirIterImpl {
virtual ~DirIterImpl();
/// Sets \c CurrentEntry to the next entry in the directory on success,
- /// or returns a system-defined \c error_code.
+ /// to directory_entry() at end, or returns a system-defined \c error_code.
virtual std::error_code increment() = 0;
- Status CurrentEntry;
+ directory_entry CurrentEntry;
};
} // namespace detail
@@ -151,7 +160,7 @@
directory_iterator(std::shared_ptr<detail::DirIterImpl> I)
: Impl(std::move(I)) {
assert(Impl.get() != nullptr && "requires non-null implementation");
- if (!Impl->CurrentEntry.isStatusKnown())
+ if (Impl->CurrentEntry.Path.empty())
Impl.reset(); // Normalize the end iterator to Impl == nullptr.
}
@@ -162,17 +171,17 @@
directory_iterator &increment(std::error_code &EC) {
assert(Impl && "attempting to increment past end");
EC = Impl->increment();
- if (!Impl->CurrentEntry.isStatusKnown())
+ if (Impl->CurrentEntry.Path.empty())
Impl.reset(); // Normalize the end iterator to Impl == nullptr.
return *this;
}
- const Status &operator*() const { return Impl->CurrentEntry; }
- const Status *operator->() const { return &Impl->CurrentEntry; }
+ const directory_entry &operator*() const { return Impl->CurrentEntry; }
+ const directory_entry *operator->() const { return &Impl->CurrentEntry; }
bool operator==(const directory_iterator &RHS) const {
if (Impl && RHS.Impl)
- return Impl->CurrentEntry.equivalent(RHS.Impl->CurrentEntry);
+ return Impl->CurrentEntry.Path == RHS.Impl->CurrentEntry.Path;
return !Impl && !RHS.Impl;
}
bool operator!=(const directory_iterator &RHS) const {
@@ -201,8 +210,8 @@
/// Equivalent to operator++, with an error code.
recursive_directory_iterator &increment(std::error_code &EC);
- const Status &operator*() const { return *State->top(); }
- const Status *operator->() const { return &*State->top(); }
+ const directory_entry &operator*() const { return *State->top(); }
+ const directory_entry *operator->() const { return &*State->top(); }
bool operator==(const recursive_directory_iterator &Other) const {
return State == Other.State; // identity
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
