[PATCH] D52549: [VFS] Add a VFS wrapper which avoids opening nonexistent files by reading dirs.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 28 01:50:42 PDT 2018


sammccall marked 2 inline comments as done.
sammccall added a comment.

Though logs looked good, actual profiles and straces shows we're still stat()ing every file in listed directories.
https://reviews.llvm.org/D44960/https://reviews.llvm.org/rL329338 is the culprit, will make a new patch to fix that.



================
Comment at: lib/Basic/AvoidStatsVFS.cpp:44
+
+class StatLessFS : public ProxyFileSystem {
+public:
----------------
ioeric wrote:
> `LessStatFS` sounds more accurate lol
Haha, renamed to AvoidStatsVFS to match the filename.
(Happy if you prefer another name but they should be consistent I think)


================
Comment at: lib/Basic/AvoidStatsVFS.cpp:186
+  //   - NormPath is a directory whose children can't be listed
+  bool populateCacheForDir(StringRef NormPath) {
+    // First, just see if we have any work to do.
----------------
ioeric wrote:
> Is there any overhead for reading all parent directories, e.g. when directories are large? Or would they be read anyway somewhere else?
There are typically fewer ancestors than children, so this is not too bad.
That said, for `/a/long/path/that/doesnt/fork/file.{cc,h}` this is suboptimal.

Tweaked the threshold logic so it properly "cascades": if you access both `file.h` and `file.cc` then it'll read `.../fork/` but not the parent directory yet. (See the new logic around `AllowIO`)


Repository:
  rC Clang

https://reviews.llvm.org/D52549





More information about the cfe-commits mailing list