[clangd] Skip function bodies when building the preamble

[ilya-biryukov]

Summary

To make building preambles faster and keep them smaller.

[ilya-biryukov]

I haven't done proper benchmarks, but here are some numbers from running this on my machine:

Sizes of preamble with (before the change) and without (after the change) functon bodies:

File	Before	After
ClangdUnit.cpp	49.58MB	34.79MB
SemaDecl.cpp	47.03MB	31.51MB
All C++11 stl includes	13.15MB	8.75M

Time to build the preamble (just a single run, not a proper benchmark):

File	Before	After
ClangdUnit.cpp	4.39s	2.49s
SemaDecl.cpp	4.28s	2.29s
All C++11 stl includes	1.40s	0.80s

Time to build AST and provide diagnostics after preamble is ready (just a single run, not a proper benchmark):

File	Before	After
ClangdUnit.cpp	1.75s	0.18s
SemaDecl.cpp	2.30s	0.74s

[sammccall]

Those are some impressive numbers!
Excited to see this land, thanks for tracking down all the weird problems.

clangd/ClangdUnit.cpp

536 ↗

(On Diff #127891)

Nit: I think the comment/assert focus too much on the 'what' at the expense of the 'why' - just the first sentence here is probably enough (without the second and without the assert).

And below, something like:

// For the main file, we do want the AST and diagnostics for function bodies.

[ilya-biryukov]

• Updated a comment

clangd/ClangdUnit.cpp

536 ↗

(On Diff #127891)

That looks better. Updated the comments, thanks.

[nik]

Hmm, could libclang profit from something like this, too? (or does it already?)

[ilya-biryukov]

In D41495#965627, @nik wrote:

Hmm, could libclang profit from something like this, too? (or does it already?)

It could. The problem with ASTUnit is that it returns all diagnostics from store_diag_begin/end, not just the ones from the current file.
Skipping function bodies would mean no diagnostics from the preamble. Which may or may not be fine, I'm not sure how important those diagnostics are for the clients.

clang-tools-extra/trunk/clangd/ClangdUnit.cpp

auto DoRebuildPreamble =
 
       trace::Span Tracer(Ctx, "Preamble");
       SPAN_ATTACH(Tracer, "File", That->FileName);
       std::vector<DiagWithFixIts> PreambleDiags;
       StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags);
       IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
           CompilerInstance::createDiagnostics(
               &CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false);
+
+      // Skip function bodies when building the preamble to speed up building
+      // the preamble and make it smaller.
+      assert(!CI->getFrontendOpts().SkipFunctionBodies);
+      CI->getFrontendOpts().SkipFunctionBodies = true;
+
       CppFilePreambleCallbacks SerializedDeclsCollector;
       auto BuiltPreamble = PrecompiledPreamble::Build(
           *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs,
           /*StoreInMemory=*/That->StorePreamblesInMemory,
           SerializedDeclsCollector);
 
+      // When building the AST for the main file, we do want the function
+      // bodies.
+      CI->getFrontendOpts().SkipFunctionBodies = false;
+
       if (BuiltPreamble) {
         log(Ctx, "Built preamble of size " + Twine(BuiltPreamble->getSize()) +
                      " for file " + Twine(That->FileName));
 
         return std::make_shared<PreambleData>(
             std::move(*BuiltPreamble),
             SerializedDeclsCollector.takeTopLevelDeclIDs(),
             std::move(PreambleDiags));