I'm interested but I don't have enough free time these days. So if anyone wants to take it from there, feel free to reuse my commits (or not).
If I find myself with some free time and nobody else is working on this, I'll give it a shot

• clangd: rename HeaderInsertion and IncludeInsertion, read value from Config

@nridge after rebasing this change for several months, I find it less and less useful. It seems both clangd and the IDE I'm using are making significant improvements that make this changes less and less impactful.
I closed this revision as I feel that it now passed the threshold where the annoyance of maintaining it is greater than the return I'm getting from it

Small correction

Resolve conflicts, fix formatting, simplify patch

This is a revisit of D97417 which is a bit out of date and fell into oblivion.
I've been using a forked version of clangd including this change for months on my day-to-day work and it does significantly improve my experience.
Some numbers showing the kind of improvement you can expect from this change can be found in D97417, they are still relevant.

• Rebase
• Add cli argument to set the cache size
• Reduce default cache size to 1 (to allow fast open/close/reopen)

Have you guys been giving some thoughts to that patch ? I've been using it in my daily work since I submitted the patch, and I'd not go back

fix tests

fix bad arc diff

rebase on main, fix formatting

I am much more afraid of providing bad results than I am afraid of degrading performance. I mean, eventually the "real" preamble is built, and the results are just as correct as before, but it may yield incorrect results until we invalidate the AST.
That is especially the case as I'm not very experienced in compiler design and I don't have a deep understanding of what happens behind the curtains when using a PCH.
There are probably cases where we degrade perf, but overall my experience with this patch is overwhelmingly positive, especially for "long coding sessions".

• [clangd] make more compile commands compatible
• [clangd] ignore incompatible preamble

The hardcoded 5 premables can indeed be changed, I did not want to waste time on coding a configuration logic at such an early stage.

I applied your suggestion as I agree with both. I chose to use Opts.AsyncThreadsCount instead of a hard-coded constant. This way the "formatting speed" will grow as the user allow more parallelism, which IMO makes sense.

• [clangd] fix nits

You're right, I probably got carried away for the sake of completeness. Allocating extra memory for a feature nobody is going to use is definitely not worth it. Though maybe we can take advantage of what's already been done and if we remove the RevRefs map, and pay the full price of iterating all refs when refersTo is called, we still have this feature. Although slower, it shouldn't take more than a few 100's of milliseconds even on a big project. Which means the feature is usable, at least for most small to medium sized projects, at no extra cost for people who do not use it.

If you look back at my original comment, you'll notice I originally trimmed the memory after FileSymbols::buildIndex which seemed to be the "end of the peak" memory usage, the goal was to counteract exactly what you describe: a huge memory usage when clangd warms-up and parses a lot of files in parallel.
I wonder if we can find a middle ground. The solution we finally adopted also has the advantage of being less intrusive, which is definitely great considering the nature of the issue

Hey, don't worry about the delay, I won't have as much time on my hands anymore anyway.

New year's ping ?

Ah ! It's been done already !
https://reviews.llvm.org/D93600

• Fix keep preamble command line option
• Fix tests

Note: Build fails due to clang-tidy warnings in tests. I chose to keep the naming consistent with what's already in place rather than fix the clang-tidy warning.

• Smaller reversed refs memory usage
• Fix Dex::estimateMemoryUsage

• Fix tests

Small nits:

• operator() is not const but Next is mutable, seems to me you intended to have operator() as const
• memory orders for the atomic operations can probably be downgraded to std::memory_order_acquire/std::memory_order_acq_rel. I think the first load can even be relaxed but that I'm always careful with these

The log is back, I renamed the CMake option and the command line option and reduced the number of preprocessor directives.
I chose not to modify the files for the gn build system, I'd rather not break it.

I guess I'll need to ask for commit access

• dont capitalize first letter of status

Here is the latest version, I believe it should mostly look like what you expect. Let me know if there is anything that you'd like me to change, it's totally fine to have a few more iterations on this diff.

• Move platform specific code to ClangdMain
• Generic memory cleanup callback in ClangdLSPServer

Sorry to mess you around like this, but I think I might have jumped the gun in asking this to be abstracted away in Process.h.

No problem, there is not much code anyway. The thought process behind the "right" solution is by far the most important.

Interestingly clangd held rock solid at 1.0GB recorded memory usage throughout.

• Fix clang-tidy errors

Looks good to me, it removes a lot of duplication and corner cases.

• Add MallocTrim to llvm::sys::Process
• Implement malloc trimming in ClangdLSPServer

Fix ExpandAutoType to not expand undeduced auto

You can land this if it is still fine after my final update.

• Verify documentation in hover test on 'auto'
• Fixed comments
• Converted raw string to string to avoid trailing whitespace

I updated this diff according to your review. There are a few things that may still need change but I need your input, see my other comments :)

Periodic malloc_trim seems like it solves a real problem, one I don't personally fully understand but we may never do - I feel convinced we should use it anyway. It probably has some overhead, who knows how much.

It does indeed look like facing the problem from the wrong side (fight the end result instead of finding the root cause) but at that point it does solve an annoying problem. As I investigated this, I found that replacing DenseMap and StringMap wtih std::map mitigates the issue. I think allocating huge chunks of contiguous memory temporarily make this issue worse. Not very useful, but hey, I'm just sharing what I observed. As I previously said, resolving this problem by changing containers or allocators is a lot of work and is not guaranteed to actually solve the issue.

Use malloc_trim only if available

Side note is this memory failing to free behaviour observed on windows?

We can't easily add new SymbolKinds (it's an enum we don't own, which we should fix, but that's a yak-shave). But TypeAlias almost fits these roles for auto.

• Rebase on master after D92041
• Remove the usage of the "Documentation" field
• Use the TypeAlias Kind on auto and decltype
• Move code related to hover on this in a new function
• Update hover on this to be consistent with this change

So I'm leaning towards defining these behaviors for pointers and containers rather than templates per se. There's a nice symmetry with the raw pointer and array types that this patch.
(We have some precedent and code for detecting smart pointers, see getPointeeType() in FindTarget.cpp.)

Ah good catch, I agree this is the behavior we want.

I've updated the patch with your latest comments.

• Reintroduce test with a FIXME comment
• locateSymbolForType returns a vector instead of an optional

• Remove the patches affecting hover
• Add tests in ASTTests.cpp to test the behavior of getDeducedType for auto and decltype
• Add missing comment
• Create a locateSymbolForType function (which uses targetDecl)
• Remove the unit tests that were testing undesirable behaviors

Not sure what's wrong with the unit test "x64 windows > LLVM.CodeGen/XCore::threads.ll", I don't see how it's related to my patch

I've fixed the failing unittests, added some more
to test the new behavior, and fixed some bugs in
the new code.