Page MenuHomePhabricator

[clangd] Skip function bodies when building the preamble
ClosedPublic

Authored by ilya-biryukov on Dec 21 2017, 8:10 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ilya-biryukov created this revision.Dec 21 2017, 8:10 AM

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:

FileBeforeAfter
ClangdUnit.cpp49.58MB34.79MB
SemaDecl.cpp47.03MB31.51MB
All C++11 stl includes13.15MB8.75M

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

FileBeforeAfter
ClangdUnit.cpp4.39s2.49s
SemaDecl.cpp4.28s2.29s
All C++11 stl includes1.40s0.80s

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

FileBeforeAfter
ClangdUnit.cpp1.75s0.18s
SemaDecl.cpp2.30s0.74s
sammccall accepted this revision.Dec 21 2017, 11:29 PM

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.
This revision is now accepted and ready to land.Dec 21 2017, 11:29 PM
ilya-biryukov marked an inline comment as done.
  • Updated a comment
clangd/ClangdUnit.cpp
536 ↗(On Diff #127891)

That looks better. Updated the comments, thanks.

This revision was automatically updated to reflect the committed changes.
nik added a subscriber: nik.Jan 2 2018, 12:41 AM

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

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.