[clang] [llvm] [clang][deps] Overload `Filesystem::exists` in `DependencyScanningFilesystem` to have it use cached `status` (PR #88152)

Alexandre Ganea via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 10 14:08:58 PDT 2024


aganea wrote:

> This patch doesn`t improve my usage, it seems I'm hitting a different codepath than you do. I'll investigate.

Ah I see, the commit https://github.com/llvm/llvm-project/commit/b768a8c1db85b9e84fd8b356570a3a8fbe37acf6 didn't make it in time for the 18.x branch. The issue I'm seeing here is with regular C++ code, no PCH, no modules. It is related to `shouldCacheStatFailures()` which currently in `release/18.x` happens to return `false` for directories. The searching behavior for `#include` files with `clang-cl` (not sure about plain clang) is essentially to query every include path passed on the command-line. That creates lots of OS `status()` calls, like shown in my profile above. Going back to the previous behavior as on `main` solves my issue, ie:
```
static bool shouldCacheStatFailures(StringRef Filename) {
  StringRef Ext = llvm::sys::path::extension(Filename);
  if (Ext.empty())
    return false; // This may be the module cache directory.
  return true;
}
```
As an order of magnitude, the Unreal Engine target I'm testing consists of 17,801 TUs, generating 1,2 billion status() calls in total (by counting the calls to `FileManager::getStatValue`) out of which  8,3 millions are real OS calls after the change above (by counting the calls to `llvm::sys::windows::status()`.

Not sure @jansvoboda11 perhaps if we want to cherry pick b768a8c1db85b9e84fd8b356570a3a8fbe37acf6 on `release/18.x`? Or should we land just a simple PR with just the function change above?

https://github.com/llvm/llvm-project/pull/88152


More information about the cfe-commits mailing list