Skip to content

Commit 0ae0056

Browse files
sam-mccallsam-mccall
committed
[VFS] vfs::directory_iterator yields path and file type instead of full Status
Summary: 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 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. Reviewers: bkramer Subscribers: fedor.sergeev, cfe-commits Differential Revision: https://reviews.llvm.org/D51921 llvm-svn: 342232
1 parent 200cd74 commit 0ae0056

File tree

10 files changed

+116
-108
lines changed

10 files changed

+116
-108
lines changed

‎clang/include/clang/Basic/VirtualFileSystem.h

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,21 @@ class File {
126126
virtual std::error_code close() = 0;
127127
};
128128

129+
/// A member of a directory, yielded by a directory_iterator.
130+
/// Only information available on most platforms is included.
131+
class directory_entry {
132+
std::string Path;
133+
llvm::sys::fs::file_type Type;
134+
135+
public:
136+
directory_entry() = default;
137+
directory_entry(std::string Path, llvm::sys::fs::file_type Type)
138+
: Path(std::move(Path)), Type(Type) {}
139+
140+
llvm::StringRef path() const { return Path; }
141+
llvm::sys::fs::file_type type() const { return Type; }
142+
};
143+
129144
namespace detail {
130145

131146
/// An interface for virtual file systems to provide an iterator over the
@@ -134,10 +149,10 @@ struct DirIterImpl {
134149
virtual ~DirIterImpl();
135150

136151
/// Sets \c CurrentEntry to the next entry in the directory on success,
137-
/// or returns a system-defined \c error_code.
152+
/// to directory_entry() at end, or returns a system-defined \c error_code.
138153
virtual std::error_code increment() = 0;
139154

140-
Status CurrentEntry;
155+
directory_entry CurrentEntry;
141156
};
142157

143158
} // namespace detail
@@ -151,7 +166,7 @@ class directory_iterator {
151166
directory_iterator(std::shared_ptr<detail::DirIterImpl> I)
152167
: Impl(std::move(I)) {
153168
assert(Impl.get() != nullptr && "requires non-null implementation");
154-
if (!Impl->CurrentEntry.isStatusKnown())
169+
if (Impl->CurrentEntry.path().empty())
155170
Impl.reset(); // Normalize the end iterator to Impl == nullptr.
156171
}
157172

@@ -162,17 +177,17 @@ class directory_iterator {
162177
directory_iterator &increment(std::error_code &EC) {
163178
assert(Impl && "attempting to increment past end");
164179
EC = Impl->increment();
165-
if (!Impl->CurrentEntry.isStatusKnown())
180+
if (Impl->CurrentEntry.path().empty())
166181
Impl.reset(); // Normalize the end iterator to Impl == nullptr.
167182
return *this;
168183
}
169184

170-
const Status &operator*() const { return Impl->CurrentEntry; }
171-
const Status *operator->() const { return &Impl->CurrentEntry; }
185+
const directory_entry &operator*() const { return Impl->CurrentEntry; }
186+
const directory_entry *operator->() const { return &Impl->CurrentEntry; }
172187

173188
bool operator==(const directory_iterator &RHS) const {
174189
if (Impl && RHS.Impl)
175-
return Impl->CurrentEntry.equivalent(RHS.Impl->CurrentEntry);
190+
return Impl->CurrentEntry.path() == RHS.Impl->CurrentEntry.path();
176191
return !Impl && !RHS.Impl;
177192
}
178193
bool operator!=(const directory_iterator &RHS) const {
@@ -201,8 +216,8 @@ class recursive_directory_iterator {
201216
/// Equivalent to operator++, with an error code.
202217
recursive_directory_iterator &increment(std::error_code &EC);
203218

204-
const Status &operator*() const { return *State->top(); }
205-
const Status *operator->() const { return &*State->top(); }
219+
const directory_entry &operator*() const { return *State->top(); }
220+
const directory_entry *operator->() const { return &*State->top(); }
206221

207222
bool operator==(const recursive_directory_iterator &Other) const {
208223
return State == Other.State; // identity

‎clang/lib/Basic/VirtualFileSystem.cpp

Lines changed: 37 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -315,27 +315,16 @@ class RealFSDirIter : public clang::vfs::detail::DirIterImpl {
315315

316316
public:
317317
RealFSDirIter(const Twine &Path, std::error_code &EC) : Iter(Path, EC) {
318-
if (Iter != llvm::sys::fs::directory_iterator()) {
319-
llvm::sys::fs::file_status S;
320-
std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
321-
CurrentEntry = Status::copyWithNewName(S, Iter->path());
322-
if (!EC)
323-
EC = ErrorCode;
324-
}
318+
if (Iter != llvm::sys::fs::directory_iterator())
319+
CurrentEntry = directory_entry(Iter->path(), Iter->type());
325320
}
326321

327322
std::error_code increment() override {
328323
std::error_code EC;
329324
Iter.increment(EC);
330-
if (Iter == llvm::sys::fs::directory_iterator()) {
331-
CurrentEntry = Status();
332-
} else {
333-
llvm::sys::fs::file_status S;
334-
std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
335-
CurrentEntry = Status::copyWithNewName(S, Iter->path());
336-
if (!EC)
337-
EC = ErrorCode;
338-
}
325+
CurrentEntry = (Iter == llvm::sys::fs::directory_iterator())
326+
? directory_entry()
327+
: directory_entry(Iter->path(), Iter->type());
339328
return EC;
340329
}
341330
};
@@ -446,11 +435,11 @@ class OverlayFSDirIterImpl : public clang::vfs::detail::DirIterImpl {
446435
while (true) {
447436
std::error_code EC = incrementDirIter(IsFirstTime);
448437
if (EC || CurrentDirIter == directory_iterator()) {
449-
CurrentEntry = Status();
438+
CurrentEntry = directory_entry();
450439
return EC;
451440
}
452441
CurrentEntry = *CurrentDirIter;
453-
StringRef Name = llvm::sys::path::filename(CurrentEntry.getName());
442+
StringRef Name = llvm::sys::path::filename(CurrentEntry.path());
454443
if (SeenNames.insert(Name).second)
455444
return EC; // name not seen before
456445
}
@@ -850,11 +839,21 @@ class InMemoryDirIterator : public clang::vfs::detail::DirIterImpl {
850839
if (I != E) {
851840
SmallString<256> Path(RequestedDirName);
852841
llvm::sys::path::append(Path, I->second->getFileName());
853-
CurrentEntry = detail::getNodeStatus(I->second.get(), Path);
842+
sys::fs::file_type Type;
843+
switch (I->second->getKind()) {
844+
case detail::IME_File:
845+
case detail::IME_HardLink:
846+
Type = sys::fs::file_type::regular_file;
847+
break;
848+
case detail::IME_Directory:
849+
Type = sys::fs::file_type::directory_file;
850+
break;
851+
}
852+
CurrentEntry = directory_entry(Path.str(), Type);
854853
} else {
855854
// When we're at the end, make CurrentEntry invalid and DirIterImpl will
856855
// do the rest.
857-
CurrentEntry = Status();
856+
CurrentEntry = directory_entry();
858857
}
859858
}
860859

@@ -1010,17 +1009,14 @@ class RedirectingFileEntry : public Entry {
10101009
static bool classof(const Entry *E) { return E->getKind() == EK_File; }
10111010
};
10121011

1013-
class RedirectingFileSystem;
1014-
10151012
class VFSFromYamlDirIterImpl : public clang::vfs::detail::DirIterImpl {
10161013
std::string Dir;
1017-
RedirectingFileSystem &FS;
10181014
RedirectingDirectoryEntry::iterator Current, End;
10191015

10201016
std::error_code incrementImpl();
10211017

10221018
public:
1023-
VFSFromYamlDirIterImpl(const Twine &Path, RedirectingFileSystem &FS,
1019+
VFSFromYamlDirIterImpl(const Twine &Path,
10241020
RedirectingDirectoryEntry::iterator Begin,
10251021
RedirectingDirectoryEntry::iterator End,
10261022
std::error_code &EC);
@@ -1184,8 +1180,8 @@ class RedirectingFileSystem : public vfs::FileSystem {
11841180
}
11851181

11861182
auto *D = cast<RedirectingDirectoryEntry>(*E);
1187-
return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(Dir,
1188-
*this, D->contents_begin(), D->contents_end(), EC));
1183+
return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(
1184+
Dir, D->contents_begin(), D->contents_end(), EC));
11891185
}
11901186

11911187
void setExternalContentsPrefixDir(StringRef PrefixDir) {
@@ -2079,10 +2075,9 @@ void YAMLVFSWriter::write(llvm::raw_ostream &OS) {
20792075
}
20802076

20812077
VFSFromYamlDirIterImpl::VFSFromYamlDirIterImpl(
2082-
const Twine &_Path, RedirectingFileSystem &FS,
2083-
RedirectingDirectoryEntry::iterator Begin,
2078+
const Twine &_Path, RedirectingDirectoryEntry::iterator Begin,
20842079
RedirectingDirectoryEntry::iterator End, std::error_code &EC)
2085-
: Dir(_Path.str()), FS(FS), Current(Begin), End(End) {
2080+
: Dir(_Path.str()), Current(Begin), End(End) {
20862081
EC = incrementImpl();
20872082
}
20882083

@@ -2096,23 +2091,21 @@ std::error_code VFSFromYamlDirIterImpl::incrementImpl() {
20962091
while (Current != End) {
20972092
SmallString<128> PathStr(Dir);
20982093
llvm::sys::path::append(PathStr, (*Current)->getName());
2099-
llvm::ErrorOr<vfs::Status> S = FS.status(PathStr);
2100-
if (!S) {
2101-
// Skip entries which do not map to a reliable external content.
2102-
if (FS.ignoreNonExistentContents() &&
2103-
S.getError() == llvm::errc::no_such_file_or_directory) {
2104-
++Current;
2105-
continue;
2106-
} else {
2107-
return S.getError();
2108-
}
2094+
sys::fs::file_type Type;
2095+
switch ((*Current)->getKind()) {
2096+
case EK_Directory:
2097+
Type = sys::fs::file_type::directory_file;
2098+
break;
2099+
case EK_File:
2100+
Type = sys::fs::file_type::regular_file;
2101+
break;
21092102
}
2110-
CurrentEntry = *S;
2103+
CurrentEntry = directory_entry(PathStr.str(), Type);
21112104
break;
21122105
}
21132106

21142107
if (Current == End)
2115-
CurrentEntry = Status();
2108+
CurrentEntry = directory_entry();
21162109
return {};
21172110
}
21182111

@@ -2130,10 +2123,10 @@ vfs::recursive_directory_iterator::recursive_directory_iterator(FileSystem &FS_,
21302123
vfs::recursive_directory_iterator &
21312124
recursive_directory_iterator::increment(std::error_code &EC) {
21322125
assert(FS && State && !State->empty() && "incrementing past end");
2133-
assert(State->top()->isStatusKnown() && "non-canonical end iterator");
2126+
assert(!State->top()->path().empty() && "non-canonical end iterator");
21342127
vfs::directory_iterator End;
2135-
if (State->top()->isDirectory()) {
2136-
vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
2128+
if (State->top()->type() == sys::fs::file_type::directory_file) {
2129+
vfs::directory_iterator I = FS->dir_begin(State->top()->path(), EC);
21372130
if (I != End) {
21382131
State->push(I);
21392132
return *this;

‎clang/lib/Driver/ToolChains/BareMetal.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ void BareMetal::AddClangCXXStdlibIncludeArgs(
122122
for (vfs::directory_iterator LI =
123123
getDriver().getVFS().dir_begin(Dir.str(), EC), LE;
124124
!EC && LI != LE; LI = LI.increment(EC)) {
125-
StringRef VersionText = llvm::sys::path::filename(LI->getName());
125+
StringRef VersionText = llvm::sys::path::filename(LI->path());
126126
auto CandidateVersion = Generic_GCC::GCCVersion::Parse(VersionText);
127127
if (CandidateVersion.Major == -1)
128128
continue;

‎clang/lib/Driver/ToolChains/Gnu.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1764,7 +1764,7 @@ void Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
17641764
std::error_code EC;
17651765
for (vfs::directory_iterator LI = D.getVFS().dir_begin(PrefixDir, EC), LE;
17661766
!EC && LI != LE; LI = LI.increment(EC)) {
1767-
StringRef VersionText = llvm::sys::path::filename(LI->getName());
1767+
StringRef VersionText = llvm::sys::path::filename(LI->path());
17681768
GCCVersion CandidateVersion = GCCVersion::Parse(VersionText);
17691769

17701770
// Filter out obviously bad entries.
@@ -2209,17 +2209,17 @@ void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
22092209
LI = D.getVFS().dir_begin(LibDir + "/" + LibSuffix, EC),
22102210
LE;
22112211
!EC && LI != LE; LI = LI.increment(EC)) {
2212-
StringRef VersionText = llvm::sys::path::filename(LI->getName());
2212+
StringRef VersionText = llvm::sys::path::filename(LI->path());
22132213
GCCVersion CandidateVersion = GCCVersion::Parse(VersionText);
22142214
if (CandidateVersion.Major != -1) // Filter obviously bad entries.
2215-
if (!CandidateGCCInstallPaths.insert(LI->getName()).second)
2215+
if (!CandidateGCCInstallPaths.insert(LI->path()).second)
22162216
continue; // Saw this path before; no need to look at it again.
22172217
if (CandidateVersion.isOlderThan(4, 1, 1))
22182218
continue;
22192219
if (CandidateVersion <= Version)
22202220
continue;
22212221

2222-
if (!ScanGCCForMultilibs(TargetTriple, Args, LI->getName(),
2222+
if (!ScanGCCForMultilibs(TargetTriple, Args, LI->path(),
22232223
NeedsBiarchSuffix))
22242224
continue;
22252225

‎clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,10 @@ static void collectIncludePCH(CompilerInstance &CI,
185185
// used here since we're not interested in validating the PCH at this time,
186186
// but only to check whether this is a file containing an AST.
187187
if (!ASTReader::readASTFileControlBlock(
188-
Dir->getName(), FileMgr, CI.getPCHContainerReader(),
188+
Dir->path(), FileMgr, CI.getPCHContainerReader(),
189189
/*FindModuleFileExtensions=*/false, Validator,
190190
/*ValidateDiagnosticOptions=*/false))
191-
MDC->addFile(Dir->getName());
191+
MDC->addFile(Dir->path());
192192
}
193193
}
194194

‎clang/lib/Frontend/FrontendAction.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -347,12 +347,12 @@ static std::error_code collectModuleHeaderIncludes(
347347
Dir != End && !EC; Dir.increment(EC)) {
348348
// Check whether this entry has an extension typically associated with
349349
// headers.
350-
if (!llvm::StringSwitch<bool>(llvm::sys::path::extension(Dir->getName()))
351-
.Cases(".h", ".H", ".hh", ".hpp", true)
352-
.Default(false))
350+
if (!llvm::StringSwitch<bool>(llvm::sys::path::extension(Dir->path()))
351+
.Cases(".h", ".H", ".hh", ".hpp", true)
352+
.Default(false))
353353
continue;
354354

355-
const FileEntry *Header = FileMgr.getFile(Dir->getName());
355+
const FileEntry *Header = FileMgr.getFile(Dir->path());
356356
// FIXME: This shouldn't happen unless there is a file system race. Is
357357
// that worth diagnosing?
358358
if (!Header)
@@ -365,7 +365,7 @@ static std::error_code collectModuleHeaderIncludes(
365365

366366
// Compute the relative path from the directory to this file.
367367
SmallVector<StringRef, 16> Components;
368-
auto PathIt = llvm::sys::path::rbegin(Dir->getName());
368+
auto PathIt = llvm::sys::path::rbegin(Dir->path());
369369
for (int I = 0; I != Dir.level() + 1; ++I, ++PathIt)
370370
Components.push_back(*PathIt);
371371
SmallString<128> RelativeHeader(UmbrellaDir.NameAsWritten);
@@ -696,10 +696,10 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
696696
Dir != DirEnd && !EC; Dir.increment(EC)) {
697697
// Check whether this is an acceptable AST file.
698698
if (ASTReader::isAcceptableASTFile(
699-
Dir->getName(), FileMgr, CI.getPCHContainerReader(),
699+
Dir->path(), FileMgr, CI.getPCHContainerReader(),
700700
CI.getLangOpts(), CI.getTargetOpts(), CI.getPreprocessorOpts(),
701701
SpecificModuleCachePath)) {
702-
PPOpts.ImplicitPCHInclude = Dir->getName();
702+
PPOpts.ImplicitPCHInclude = Dir->path();
703703
Found = true;
704704
break;
705705
}

‎clang/lib/Lex/HeaderSearch.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,17 +1574,17 @@ void HeaderSearch::collectAllModules(SmallVectorImpl<Module *> &Modules) {
15741574
vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
15751575
for (vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
15761576
Dir != DirEnd && !EC; Dir.increment(EC)) {
1577-
if (llvm::sys::path::extension(Dir->getName()) != ".framework")
1577+
if (llvm::sys::path::extension(Dir->path()) != ".framework")
15781578
continue;
15791579

15801580
const DirectoryEntry *FrameworkDir =
1581-
FileMgr.getDirectory(Dir->getName());
1581+
FileMgr.getDirectory(Dir->path());
15821582
if (!FrameworkDir)
15831583
continue;
15841584

15851585
// Load this framework module.
1586-
loadFrameworkModule(llvm::sys::path::stem(Dir->getName()),
1587-
FrameworkDir, IsSystem);
1586+
loadFrameworkModule(llvm::sys::path::stem(Dir->path()), FrameworkDir,
1587+
IsSystem);
15881588
}
15891589
continue;
15901590
}
@@ -1642,10 +1642,9 @@ void HeaderSearch::loadSubdirectoryModuleMaps(DirectoryLookup &SearchDir) {
16421642
vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
16431643
for (vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
16441644
Dir != DirEnd && !EC; Dir.increment(EC)) {
1645-
bool IsFramework =
1646-
llvm::sys::path::extension(Dir->getName()) == ".framework";
1645+
bool IsFramework = llvm::sys::path::extension(Dir->path()) == ".framework";
16471646
if (IsFramework == SearchDir.isFramework())
1648-
loadModuleMapFile(Dir->getName(), SearchDir.isSystemHeaderDirectory(),
1647+
loadModuleMapFile(Dir->path(), SearchDir.isSystemHeaderDirectory(),
16491648
SearchDir.isFramework());
16501649
}
16511650

‎clang/lib/Lex/ModuleMap.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,11 +1001,11 @@ Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
10011001
for (vfs::directory_iterator Dir = FS.dir_begin(SubframeworksDirName, EC),
10021002
DirEnd;
10031003
Dir != DirEnd && !EC; Dir.increment(EC)) {
1004-
if (!StringRef(Dir->getName()).endswith(".framework"))
1004+
if (!StringRef(Dir->path()).endswith(".framework"))
10051005
continue;
10061006

10071007
if (const DirectoryEntry *SubframeworkDir =
1008-
FileMgr.getDirectory(Dir->getName())) {
1008+
FileMgr.getDirectory(Dir->path())) {
10091009
// Note: as an egregious but useful hack, we use the real path here and
10101010
// check whether it is actually a subdirectory of the parent directory.
10111011
// This will not be the case if the 'subframework' is actually a symlink
@@ -2374,10 +2374,9 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
23742374
vfs::FileSystem &FS = *SourceMgr.getFileManager().getVirtualFileSystem();
23752375
for (vfs::recursive_directory_iterator I(FS, Dir->getName(), EC), E;
23762376
I != E && !EC; I.increment(EC)) {
2377-
if (const FileEntry *FE =
2378-
SourceMgr.getFileManager().getFile(I->getName())) {
2377+
if (const FileEntry *FE = SourceMgr.getFileManager().getFile(I->path())) {
23792378

2380-
Module::Header Header = {I->getName(), FE};
2379+
Module::Header Header = {I->path(), FE};
23812380
Headers.push_back(std::move(Header));
23822381
}
23832382
}

0 commit comments

Comments
 (0)