sammccall added a comment. Some performance measurements... (workload is PreambleCallback for clangd/XRefs.cpp in sync mode with a fixed baseline of 99b01cf28db9db1a3ec0e25367bd325b7aca6d43 <https://reviews.llvm.org/rG99b01cf28db9db1a3ec0e25367bd325b7aca6d43>, opt build without asserts)
baseline: 1.21s baseline+D98242 <https://reviews.llvm.org/D98242>: 1.25s baseline+D98242 <https://reviews.llvm.org/D98242>+D98329 <https://reviews.llvm.org/D98329>: 1.21s baseline+D98371 <https://reviews.llvm.org/D98371>: 1.02s So this patch does indeed help, and cancels out the extra work done in D98242 <https://reviews.llvm.org/D98242> with this workload at least. Nevertheless I think the speedups from D98371 <https://reviews.llvm.org/D98371> clearly justify the more invasive approach to caching, so we should do something like that instead. (We don't need both as it caches at a slightly higher level). ================ Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:792 + return HeaderToInclude[FID] = + getIncludeHeader(S, SM.getFileID(SM.getIncludeLoc(FID))); // Conservatively refuse to insert #includes to files without guards. ---------------- this is correct but subtle: if we get to here we know the symbol does not have any special handling so it's safe to cache with the file Id alone Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98329/new/ https://reviews.llvm.org/D98329 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
