[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

thakis added a comment. If it's not too much trouble, maybe you could cherry-pick and amend 8d498e08deaf6e06a578cfedb4eb259b722ac7f6 into the commit that relands this. (If not, no worries.) Repository: rG LLVM Github Mon

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss added a comment. In D136651#4067012 , @glandium wrote: > Oh yes, we're using `-DLLVM_LINK_LLVM_DYLIB=ON` too, so that would be the > main trigger. This was indeed the main trigger. Thanks for the help narrowing this down. I reverted for now. Re

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

glandium added a comment. Oh yes, we're using `-DLLVM_LINK_LLVM_DYLIB=ON` too, so that would be the main trigger. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 _

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss added a comment. In D136651#4066960 , @mstorsjo wrote: > In D136651#4066949 , @friss wrote: > >> Thanks for the report, unfortunately I have no way of testing this setup. > > Can you try a build with `-DLLVM

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

mstorsjo added a comment. In D136651#4066949 , @friss wrote: > Thanks for the report, unfortunately I have no way of testing this setup. Can you try a build with `-DLLVM_LINK_LLVM_DYLIB=ON`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss added a comment. In D136651#4064474 , @glandium wrote: > In D136651#4064260 , @glandium > wrote: > >> This broke our mac builds with errors like: >> >> ld64.lld: error: undefined symbol: CFRunLoopRun >>

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

mstorsjo added a comment. In D136651#4064474 , @glandium wrote: > In D136651#4064260 , @glandium > wrote: > >> This broke our mac builds with errors like: >> >> ld64.lld: error: undefined symbol: CFRunLoopRun >

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

glandium added a comment. In D136651#4064260 , @glandium wrote: > This broke our mac builds with errors like: > > ld64.lld: error: undefined symbol: CFRunLoopRun > >>> referenced by > tools/clang/tools/clang-stat-cache/CMakeFiles/clang-stat-cache.dir

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

glandium added a comment. This broke our mac builds with errors like: ld64.lld: error: undefined symbol: CFRunLoopRun >>> referenced by tools/clang/tools/clang-stat-cache/CMakeFiles/clang-stat-cache.dir/clang-stat-cache.cpp.o:(symbol main+0x11be) (and many more symbols) Repository: rG

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

lebedev.ri added a comment. In D136651#4063818 , @friss wrote: > In D136651#4063632 , @lebedev.ri > wrote: > >> In D136651#4063597 , @friss wrote: >> >>> In D136651#40600

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss added a comment. In D136651#4063632 , @lebedev.ri wrote: > In D136651#4063597 , @friss wrote: > >> In D136651#4060071 , @lebedev.ri >> wrote: >> >>> Docs still mis

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

lebedev.ri added a comment. In D136651#4063597 , @friss wrote: > In D136651#4060071 , @lebedev.ri > wrote: > >> Docs still missing. > > Sorry @lebedev.ri I missed your earlier comment about this. What format of

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss added a subscriber: lebedev.ri. friss added a comment. In D136651#4060071 , @lebedev.ri wrote: > Docs still missing. Sorry @lebedev.ri I missed your earlier comment about this. What format of doc would you like to see? A more elaborate comment in

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa033dbbe5c43: [Clang] Give Clang the ability to use a shared stat cache (authored by friss). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss updated this revision to Diff 489968. friss added a comment. More Windows fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 Files: clang/include/clang/Basic/DiagnosticFrontendKinds.td clang/

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

lebedev.ri added a comment. Docs still missing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss updated this revision to Diff 489914. friss added a comment. The pre-commit CI showed some test failures on Windows. Try to address these. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 Files: cl

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

benlangmuir added a comment. > Sorry for missing these. I think I addressed all of them now. Thanks, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 ___

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss added a comment. In D136651#4030008 , @benlangmuir wrote: > Re-added a few of my minor comments that I think got lost on a specific > version of the diff. Otherwise LGTM. Sorry for missing these. I think I addressed all of them now. Repository:

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss updated this revision to Diff 486711. friss added a comment. Fix Windows build (Thanks @ravi-ramaseshan) Address @benlangmuir comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 Files: clang/

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

benlangmuir accepted this revision. benlangmuir added a comment. Re-added a few of my minor comments that I think got lost on a specific version of the diff. Otherwise LGTM. Comment at: llvm/include/llvm/Support/StatCacheFileSystem.h:23 +/// A ProxyFileSystem using cached info

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss updated this revision to Diff 486466. friss added a comment. Rebase correctly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 Files: clang/include/clang/Basic/DiagnosticFrontendKinds.td clang/in

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

lebedev.ri added a comment. As someone who has no idea what this does, this seems to be missing docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 ___ cfe-co

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss added inline comments. Comment at: clang/tools/clang-stat-cache/clang-stat-cache.cpp:291 + IsCaseSensitive = + ::pathconf(TargetDirectory.c_str(), _PC_CASE_SENSITIVE) == 1; +#endif benlangmuir wrote: > Is this pathconf extension Darwin-only? I don't t

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss updated this revision to Diff 486439. friss added a comment. Address review feedback Move StatCacheFileSystem to its own file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 Files: clang/include/

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

ravi-ramaseshan requested changes to this revision. ravi-ramaseshan added inline comments. Comment at: clang/tools/clang-stat-cache/clang-stat-cache.cpp:28 +#include +#include + Some of these includes may not be available on a platform like Windows. Can you pl

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

benlangmuir requested changes to this revision. benlangmuir added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Driver/Options.td:3374 +def ivfsstatcache : JoinedOrSeparate<["-"], "ivfsstatcache">, Group, Flags<[CC1Option]>, + H

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss updated this revision to Diff 472379. friss added a comment. Fix unit test build Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 Files: clang/include/clang/Basic/DiagnosticFrontendKinds.td clang

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss updated this revision to Diff 472355. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 Files: clang/include/clang/Basic/DiagnosticFrontendKinds.td clang/include/clang/Driver/Options.td clang/incl

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss updated this revision to Diff 472321. friss added a comment. Add version number in cache file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 Files: clang/include/clang/Basic/DiagnosticFrontendKi

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

steven_wu added a comment. > Yes, we can. I pondered doing it in the original patch but didn't see a > reason this would evolve in the future. Of course, I can't predict the > future, so maybe a version field is the safe approach. I think a version number should be good enough for now. For the

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss added inline comments. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:3002 +// The format of the stat cache is (pseudo-code): +// struct stat_cache { +//char Magic[4]; // "STAT" or "Stat" steven_wu wrote: > Few comments about stats repres

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

steven_wu added inline comments. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:3002 +// The format of the stat cache is (pseudo-code): +// struct stat_cache { +//char Magic[4]; // "STAT" or "Stat" Few comments about stats representation. 1. Ca

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss updated this revision to Diff 470644. friss added a comment. Address some review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 Files: clang/include/clang/Basic/DiagnosticFrontendKinds.t

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

bnbarham added a comment. > You're correct that this overhead has been measured on implicit module > builds. As I mentioned in the commit message this saves over 20% of the > overall built time in some cases. This time is split between module > validation (which could be skipped) and HeaderSear

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss added a comment. Thanks for the initial feedback! > Mostly just skimmed so far, will hopefully have some time to look at this > more tomorrow. Out of interest, do you have any performance numbers using > this change? I assume this mainly impacts implicit modules (since I suspect > we'd a

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

bnbarham added a comment. Mostly just skimmed so far, will hopefully have some time to look at this more tomorrow. Out of interest, do you have any performance numbers using this change? I assume this mainly impacts implicit modules (since I suspect we'd also be opening the file as well anyway)

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

tschuett added inline comments. Comment at: clang/tools/clang-stat-cache/clang-stat-cache.cpp:61 + +namespace { + Sorry, but you misuse anonymous namespaces. You want static instead. https://llvm.org/docs/CodingStandards.html#anonymous-namespaces Repository:

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

friss created this revision. friss added reviewers: jansvoboda11, bnbarham, arphaman. Herald added a subscriber: hiraditya. Herald added a project: All. friss requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Every Clang i