It'd be interesting to have an estimate of how much faster this makes that initial parse.

EDIT: informal estimate: about 10% on a project of some 250 files many of which in ObjC++ (2 threads allowed for the bg. parser). This is when triggering a full reparse in an already opened session configured not to be parsed on import.
The gain may be a bit higher when multiple projects are being imported and parsed at the same time.

Replying to a remark by Milian on D17289 where this idea came up:

we use CXTranslationUnit_CreatePreambleOnFirstParse to get code completion results fast. otherwise the first code completion request would create the preamble, which felt much worse

Felt or has this been quantified - and if so are those numbers still representative?
I'm guessing that I should get that same "worse feeling" with my habit of not parsing an entire project when opening it. I never had that feeling, having to wait the additional time while KDevelop burns CPU to do things (AGAIN) I will probably never require is much worse IMHO.

It's self-evident that parsing a file is not instantaneous, there's always a lag between the moment you (re)open a file and the parser is done (it seems the parser always gets triggered even when you reopen a file). I've never really noticed this to be worse the 1st time I open a file but I guess that's because there too I know that this is to be expected. Code colouring gives good feedback here, btw.

I cannot comment on the lag before code completion becomes available, it's very rare that I need this in the interval after opening a file and locating the spot where I want to change something.

On a general note: I take it libclang doesn't provide the information used by the parser in sequential fashion (I've never noticed a "syntax colouring wave" move from top to bottom through the file)?

On my test file, time spent in clang_codeCompleteAt goes down from ~700ms

Seriously, we're complaining here about something that takes 700ms the first time?? An almost 70x speed-up sounds impressive but that reeks of a synthetic benchmark. Which also included other changes.
What I'd like to see is how much difference the user sees with or without a preamble created on first parse. If that difference is obvious it can be quantified using a screen recording and measured in frames.

Evidently there should also be some better quantification (from the patch author) of what is gained by postponing the preamble creation.

Preambles are there to speed up repeated reparsing. Most translation units won't be reparsed in a typical session. Since we pay for every preamble we create, I don't think we should create them unless we've some indication that a file needs to be parsed again. It's pretty safe to assume that a file needs to be reparsed again if we parse it for the second time. There is probably no better indicator, which is why this is the default.

The flag provides us with an advantage only in the following very narrow scenario (which is probably exactly what the benchmark is measuring):

• User opens a file, patiently waits for it to be parsed.
• Then starts editing and immediately expects code completion to jump into action.

If the user doesn't wait for the initial parse, there will be a delay anyway. Otherwise, there will only be a delay on the very first edit. Is that too much to pay for not having to serialize and store preambles for the entire project?

In D18551#401241, @mwolff wrote:

Maybe the correct fix would be to check if the document is currently open, and if so, directly create the preamble on first parse. If the document isn't open yet, then we don't need to create the preamble yet.

For practical reasons we can only keep a small number of preambles alive. They can be pretty large. When working on the LLVM code base, I observed the typical preamble to be between 30 and 60 MB in size. Having a dozen files open is probably not unusual, and then we get into trouble very soon. Also, I can't talk about others, but I often open files without editing them — KDevelop is not just good for writing code, but also reading it. We only need preambles for code that we change.

Note that this also needs to work properly when we reparse for code completion, that must create the preamble directly.

My understanding is that any reparsing triggers the preamble building.

As-is, this patch is a no-go, but I would like to see more work in this area.

The way I see it, the current situation is a no-go. LLVM has roughly 10.000 translation units. For every TU we create a preamble that is somewhere between 10–70 MB and write it to disk. Thus parsing the entire project results in 100 GB disk I/O (and that is very conservative). Almost all of this is immediately thrown away. There are also larger projects that I'm working on, roughly in the 100.000 TU range.

We should probably also create a proper benchmark for this while at it.

That might depend on the hardware and TMPDIR location, as the preambles are written there. I have /tmp on a RAM disk, but my distro's default is to have no separate file system for it. Then the preambles can actually be written to a spinning hard disk.

Aaron, don't your arguments apply to parsing an entire project on startup as well? I can easily imagine that opening and parsing entire projects of the size you mention must have a non-negligible cost regardless of all optimisation and caching that is currently implemented
There is a setting for this feature. A similar switch for the preamble thing will probably be difficult to label comprehensively but maybe a coupling can be made with the full-project-parse switch?

Probably in the sense that you create the preamble on first parse only when not parsing the entire project on import. I'd think that this should maintain the status quo for those files that the user actually opens, while not generating preambles for all the other files.

In D18551#401594, @rjvbb wrote:

Aaron, don't your arguments apply to parsing an entire project on startup as well? I can easily imagine that opening and parsing entire projects of the size you mention must have a non-negligible cost regardless of all optimisation and caching that is currently implemented

It definitely does. Sometimes I just cancel it, but sometimes I need the entire index to understand some code. (“Find uses” is very useful for that.)

Probably in the sense that you create the preamble on first parse only when not parsing the entire project on import. I'd think that this should maintain the status quo for those files that the user actually opens, while not generating preambles for all the other files.

I think that's roughly what @mwolff meant in his comment as well: to create preambles only for open files. It's definitely a nice idea, because then we can immediately do code completion for all open files. But as I mentioned, some open files are never changed, especially when a user is just browsing the code to understand it. So I could definitely live with this, but I would slightly prefer going with the default, meaning to build the preamble only on first reparse. I don't think that hurts too bad. If I'm the only one who thinks that way however, I would change the patch to create preambles only for open files.

To clarify, the patch back then was less about the first-time delay, and more about the delay being there *every* time because of wrong usage of the API. Whether the delay occurs once is arguably a bit less important.

So there should indeed be some real-world benchmarking data to do a cost/benefit analysis that includes the human-in-the-loop aspect and huge projects like the ones Aaron uses. We already have an estimated 10% gain for a full reparse of a small project after its initial import.

I'm attaching two potential useful personal patches that add timing on the project import and parsing in case anyone else wants to look at such data, the patch that allows to reparse a loaded project without reloading it is D11934 .

In D18551#402186, @brauch wrote:

To clarify, the patch back then was less about the first-time delay, and more about the delay being there *every* time because of wrong usage of the API. Whether the delay occurs once is arguably a bit less important.

That's good to hear, reading your commit message made me think that preventing the delay of the first reparse was not the goal of these changes.

again: why don't we keep this flag when the file is opened in the editor?

I'm fine with that, but would prefer never having it. Not all files that are opened will be edited, and the delay caused by creating the preamble after the first edit is probably negligible over the lifetime of a session. It would also allow us to just drop affected preambles when a header is edited — otherwise we might have to recreate all preambles that include the header.

Is Milian's suggestion different from mine of setting the flag only when projects are not parsed entirely on import (which may be easier to test for where the flag is set, I haven't checked)?

The way I understand it, the proposals are slightly different. Maybe we can summarize them as follows:

where (1) = entire project is parsed on import. Is that correct? For your proposal the first column says “never” because you don't create preambles immediately when parsing the entire project.

where (1) = entire project is parsed on import. Is that correct?

If line 3 means "files opened after (1)" then yes, that's probably correct.

In D18551#404077, @rjvbb wrote:

If line 3 means "files opened after (1)" then yes, that's probably correct.

Then I misunderstood you. You want to make it dependent on whether the file is opened after parsing the entire project, if it is parsed at all? I thought you meant it should depend on whether a user has chosen that option.

No, sorry, I am a bit confused by your notation and the fact you seem to consider only files that are open when you start a session and then probably confused myself a bit more while answering.

So:

• when "¬(1)", the clang flag is set and preambles are created as they are currently, nothing changes.
• when "(1)", the clang flag is not set, preambles are not created during the initial import except possibly for the files that opened with the session if the parser is triggered twice for those. If it is not, the preamble will be created on first change. For all other files the preamble will be created later, I presume when you open the file (because that always seems to trigger the parser for me).

In D18551#404528, @rjvbb wrote:

preambles are not created during the initial import except possibly for the files that opened with the session if the parser is triggered twice for those.

Is the parser triggered again if the entire project has been parsed already? I thought not, but I haven't looked at the code yet. If that is the case, then your and Milian's approaches should have the same effect, if I'm not missing anything.

Is the parser triggered again if the entire project has been parsed already?

I can't say that it happens systematically but neither that it does NOT happen systematically, and I mean that in general: re-opening a file you just closed.

Isn't this unavoidable BTW, given that we don't end up with preambles of all files?

In D18551#405494, @rjvbb wrote:

I can't say that it happens systematically but neither that it does NOT happen systematically, and I mean that in general: re-opening a file you just closed.

Isn't this unavoidable BTW, given that we don't end up with preambles of all files?

Yes, we don't keep these. But my understanding is that we translate Clang's index into our own (the mystical DUChain) and when opening a file it might not be necessary to reparse the file, if we still have everything we need in our own data structures. Then I'm also not sure if we actually "reparse" (i.e. call clang_reparseTranslationUnit) the translation unit that we still have laying around, but have "suspended" with clang_suspendTranslationUnit, or if we create a new one. I don't see us calling the latter function, so I assumed we just dispose the translation unit (clang_disposeTranslationUnit), meaning that we have to create it again when opening it — which would make it the first parse, not the first reparse.

In D18551#403490, @aaronpuchert wrote:

I'm fine with that, but would prefer never having it. Not all files that are opened will be edited, and the delay caused by creating the preamble after the first edit is probably negligible over the lifetime of a session. It would also allow us to just drop affected preambles when a header is edited — otherwise we might have to recreate all preambles that include the header.

Hm yes and no. Yes, many files you open due to browsing won't be edited. But once you do, performance is imo quite important. See e.g. ClangSupport::documentActivated which has explicit code to attach the AST cf. AttachASTWithoutUpdating. The AST is essentially the Clang TU representation. Without that, we can't do code completion at all. Now, imagine we open a C++ file, we realize we don't have the AST, so we trigger a parse to attach the AST. Now you do code completion. When we didn't create the preamble, we will have to reparse the file completely. And this can easily take 1-2s per file, which is *really* annoying. Also note how we just reparsed the file solely for code completion purposes!

So I really am opposed to the idea of never passing CXTranslationUnit_CreatePreambleOnFirstParse. Please, do create the preamble when we have the TU main file or its header file open in the editor.

In D18551#409551, @mwolff wrote:

When we didn't create the preamble, we will have to reparse the file completely. And this can easily take 1-2s per file, which is *really* annoying.

It only happens on the very first edit though, and will only be noticeable if code completion is required immediately after starting to edit the file. All following parses and code completions can use the precompiled preamble then. I don't think that's too bad. It's also not unusual, many applications have something like a "warmup phase" where not everything is loaded yet.

Also note how we just reparsed the file solely for code completion purposes!

You mean the preamble? Since code completion is usually invoked after some initial changes, we have to reparse anyway. What you said would only be true if we invoke code completion without changing anything first, which seems rather weird.

So I really am opposed to the idea of never passing CXTranslationUnit_CreatePreambleOnFirstParse. Please, do create the preamble when we have the TU main file or its header file open in the editor.

For all open files, or just the ones that are active? My point is that we will probably not get to a point where we can guarantee that we have preambles for all open files around, so why bother at all? Preambles are there to speed up repeated reparsing, so I think we should only keep them for files where that is actually the case, meaning that they are actively changed.

Anything beyond that would put enormous strain on the user's resources. Imagine someone editing a header that is used in other open files. We don't want to immediately reparse them every time the header is changed. So the user will always get in situations where the preamble is not immediately available. Unless we guarantee that however, using the flag is purely window-dressing.

+++

Or, you add a flag to one of the relevant settings panels. There are already a number of options that aren't exactly easy to understand for everyone, this one could be something like "Make code-completion results available faster (can require significant resources)".

In D18551#410396, @rjvbb wrote:

Or, you add a flag to one of the relevant settings panels. There are already a number of options that aren't exactly easy to understand for everyone, this one could be something like "Make code-completion results available faster (can require significant resources)".

Not so sure about that. I would rather make sure that the currently active document (meaning it has keyboard focus) is always up-to-date. Currently we don't reparse (always?) when an included file changes. That's probably way more annoying than the first code completion having some delay.

This is not something that should be configurable. The follow-up to hard-to-understand options shouldn't be to add more of them.

Well, if among developers you cannot find a solution that covers all use cases a configurable setting (applying only to setting the preamble-on-first parse flag or not) could well be the only compromise. And this one doesn't have to be hard to understand, which doesn't mean it has to (probably means that it should not) indicate exactly what it does.

I would rather make sure that the currently active document (meaning it has keyboard focus) is always up-to-date.

But that's a different issue, isn't it? Can the parser even be expected to react to chages to a headerfile that don't lead to a change notification?

way more annoying than the first code completion having some delay.

Oh yes, there are many things way more annoying than that. Like the fact that the parser can probably never know when you're done with an edit and only then start parsing instead of belching out errors about unfinished changes ;) Or when not to apply a seemingly obvious code completion which you then have to correct manually

In D18551#411309, @rjvbb wrote:

Well, if among developers you cannot find a solution that covers all use cases a configurable setting (applying only to setting the preamble-on-first parse flag or not) could well be the only compromise.

I wouldn't want to give up on a consensus just yet. I think the Clang developers have actually chosen a pretty smart default, and this additional flag incurs costs that are hard to control for hardly any benefit.

To some extent this resembles the classic latency/throughput discussion. Just with a minor twist: we increase the overall load of the program for a one-time latency reduction. Keeping unnecessary preambles is problematic regardless of whether /tmp is a RAM disk or not — if it is, we consume valuable memory without using it, if it isn't, we incur unnecessary disk I/O.

I would rather make sure that the currently active document (meaning it has keyboard focus) is always up-to-date.

But that's a different issue, isn't it? Can the parser even be expected to react to changes to a headerfile that don't lead to a change notification?

I can't say how hard it would be technically. But since we watch files anyway, we could also track dependencies and reparse active documents if a dependency has changed. Of course that's a separate issue and probably not easy to solve.

To some extent this resembles the classic latency/throughput discussion.

I'd say that is exactly what it is, not just to some extent. What else would it be?

Keeping unnecessary preambles is problematic regardless of whether `/tmp` is a RAM disk or not

Indeed, and in that light clang's solution is maybe not that smart, at least not for use as a parser for an entire project. From what I understand that preamble is just a precompiled header file, which in my experience can be tricky to set up (typically require some sort of central header that imports everything that'll be needed everywhere). Having one such preamble per file cannot but duplicate lots of data.

I can't say how hard it would be technically. But since we watch files anyway,

Separate issue, but not all systems support tracking (that many) individual files. On Mac you only get directory watching, for instance.

we could also track dependencies and reparse active documents if a dependency has changed.

I was assuming this is what happens, at least for open files. Of course that could be something done entirely by libclang internally.

In D18551#411905, @rjvbb wrote:

Indeed, and in that light clang's solution is maybe not that smart, at least not for use as a parser for an entire project. From what I understand that preamble is just a precompiled header file, which in my experience can be tricky to set up (typically require some sort of central header that imports everything that'll be needed everywhere). Having one such preamble per file cannot but duplicate lots of data.

You're right, it's a precompiled header file. I think it works very well for the intended purpose: quick reparsing on most user edits. The additional option that we use here was added in https://reviews.llvm.org/D15490 for pure code completion workloads. I don't think it's meant for us, even though we do completion as well: we also build an index, and have possibly many more files open.

we could also track dependencies and reparse active documents if a dependency has changed.

I was assuming this is what happens, at least for open files. Of course that could be something done entirely by libclang internally.

Sometimes it works, sometimes it doesn't. Often I have to mock-edit a file to force reparsing when some header has changed.

Sometimes it works, sometimes it doesn't. Often I have to mock-edit a file to force reparsing when some header has changed.

I find that just reloading the file works too (I have a shortcut for that).
Come to think of it, libclang probably doesn't bother with file-change monitoring given that it's intended primarily for use in compilers.

In D18551#410223, @aaronpuchert wrote:

In D18551#409551, @mwolff wrote:

When we didn't create the preamble, we will have to reparse the file completely. And this can easily take 1-2s per file, which is *really* annoying.

It only happens on the very first edit though, and will only be noticeable if code completion is required immediately after starting to edit the file. All following parses and code completions can use the precompiled preamble then. I don't think that's too bad. It's also not unusual, many applications have something like a "warmup phase" where not everything is loaded yet.

Yes, but hiding or minimizing the warmup phase is good. And it would be possible by having the flag active for opened files like I request.

Also note how we just reparsed the file solely for code completion purposes!

You mean the preamble? Since code completion is usually invoked after some initial changes, we have to reparse anyway. What you said would only be true if we invoke code completion without changing anything first, which seems rather weird.

I mean: We trigger a reparse when we open a document, to get hold of the clang TU to do code completion. If this *does not* create a preamble, then the following call to query code completion items - *after* editing - will create the preamble. The preamble only includes the headers of the file, not the contents of the .cpp. You usually edit the .cpp file and then you want to get code completion results immediately. Having a preamble is crucial for good performance here.

So I really am opposed to the idea of never passing CXTranslationUnit_CreatePreambleOnFirstParse. Please, do create the preamble when we have the TU main file or its header file open in the editor.

For all open files, or just the ones that are active? My point is that we will probably not get to a point where we can guarantee that we have preambles for all open files around, so why bother at all? Preambles are there to speed up repeated reparsing, so I think we should only keep them for files where that is actually the case, meaning that they are actively changed.

Lacking an proper LRU cache, enabling it for all open documents is the only option we have currently I think.

Anything beyond that would put enormous strain on the user's resources. Imagine someone editing a header that is used in other open files. We don't want to immediately reparse them every time the header is changed. So the user will always get in situations where the preamble is not immediately available. Unless we guarantee that however, using the flag is purely window-dressing.

The reparse would only happen when we trigger a new parse job. Which we - afaik - only do on activation.

In D18551#411201, @aaronpuchert wrote:

Currently we don't reparse (always?) when an included file changes. That's probably way more annoying than the first code completion having some delay.

That sounds like a serious bug, I was under the impression that we properly check the include files for updates too. But thinking more about this: Afaik we properly did that in old-cpp, but maybe kdev-clang lost that feature since we don't store the included files anymore? I have to look into this. Anyhow, unrelated to the patch at hand.

Re: reparsing reliably each time a headerfile is changed: wouldn't the use of forwarding headers increase the chance of missing a change?

In D18551#430847, @rjvbb wrote:

Re: reparsing reliably each time a headerfile is changed: wouldn't the use of forwarding headers increase the chance of missing a change?

What's a forwarding header? Do you mean something like #include <Foo> which then contains a #include <foo.h>? We obviously should use the full include chain which will cover all files, not only direct includes.

Do you mean something like `#include <Foo>` which then contains a `#include <foo.h>`?

Yes.

We obviously should use the full include chain which will cover all files, not only direct includes.

This kind of circles back to earlier discussions we had about monitoring file changes and how that's not always as simple as adding another inotify thingy. If anything it would be mostly a waste to monitor a header that only serves to include another header (except in the unlikely event the forward changes).

Is it even up to KDevelop to detect changes in all files that should trigger a reparse of a given translation unit or does libclang take care of that?

quite obviously libclang doesn't handle it

I still think this is the right change, but if there is no consensus I will implement @mwolff's suggestion. That would certainly be an improvement, but it doesn't really solve my problem. I'm currently working on Clang and have around 6 files open, which is really not a lot. I haven't changed any of them. The result:

aaron:~/src/llvm-project> ls -l /tmp/preamble-*
-rw-r--r-- 1 aaron users 73139496 Okt  9 22:27 /tmp/preamble-0e19f4.pch
-rw-r--r-- 1 aaron users 59327864 Okt  9 22:23 /tmp/preamble-2a515c.pch
-rw-r--r-- 1 aaron users 61885536 Okt  9 22:23 /tmp/preamble-2baca3.pch
-rw-r--r-- 1 aaron users 76913424 Okt  9 22:23 /tmp/preamble-9853df.pch
-rw-r--r-- 1 aaron users 64254872 Okt  9 22:23 /tmp/preamble-cbb035.pch
-rw-r--r-- 1 aaron users 64829792 Okt  9 22:24 /tmp/preamble-dd9bf1.pch
aaron:~/src/llvm-project> du -s /tmp/ 2>/dev/null
391204  /tmp/

Now I actually have /tmp as tmpfs, and so these 400 MB don't come for free. This is perfectly fine when I'm actually working on these files, but so far I have only opened KDevelop.

In D18551#412611, @mwolff wrote:

In D18551#410223, @aaronpuchert wrote:

In D18551#409551, @mwolff wrote:

When we didn't create the preamble, we will have to reparse the file completely. And this can easily take 1-2s per file, which is *really* annoying.

It only happens on the very first edit though, and will only be noticeable if code completion is required immediately after starting to edit the file. All following parses and code completions can use the precompiled preamble then. I don't think that's too bad. It's also not unusual, many applications have something like a "warmup phase" where not everything is loaded yet.

Yes, but hiding or minimizing the warmup phase is good. And it would be possible by having the flag active for opened files like I request.

Creating a preamble is actually more expensive than not creating it. So we make the initial parsing slower. This is only faster for files are actually edited.

Creating it immediately only improves the reaction time for the first edit. For that small benefit, we're paying a hefty price.

Also note how we just reparsed the file solely for code completion purposes!

You mean the preamble? Since code completion is usually invoked after some initial changes, we have to reparse anyway. What you said would only be true if we invoke code completion without changing anything first, which seems rather weird.

I mean: We trigger a reparse when we open a document, to get hold of the clang TU to do code completion. If this *does not* create a preamble, then the following call to query code completion items - *after* editing - will create the preamble. The preamble only includes the headers of the file, not the contents of the .cpp.

Well, then why don't we just skip the initial reparse, and only reparse after the first edit?

You usually edit the .cpp file and then you want to get code completion results immediately.

We're talking about a one-time delay, all further edits will have a preamble.

Having a preamble is crucial for good performance here.

I'm not suggesting to not create a preamble at all, but to create it only when we know it is needed.

I'm not suggesting to not create a preamble at all, but to create it only when we **know** it is needed.

+++
This should speed up the initial opening significantly for sessions that restore a large number of documents on start (= most of mine).

It may be worth repeating my recent observation here that under some conditions a reparse will be triggered on launch of files that are not open but mentioned in one of the session's working sets that have remained in the sessionrc file. That's probably a bug (just like the "do you want to save this modified file" alert I got the other day for a document that wasn't open at all *in the editor*). But then again I never grasped what KDevelop's working sets are supposed to do (and how), so maybe it *is* relevant here.

In D18551#544467, @aaronpuchert wrote:

I still think this is the right change, but if there is no consensus I will implement @mwolff's suggestion. That would certainly be an improvement, but it doesn't really solve my problem. I'm currently working on Clang and have around 6 files open, which is really not a lot. I haven't changed any of them. The result:

aaron:~/src/llvm-project> ls -l /tmp/preamble-*
-rw-r--r-- 1 aaron users 73139496 Okt  9 22:27 /tmp/preamble-0e19f4.pch
-rw-r--r-- 1 aaron users 59327864 Okt  9 22:23 /tmp/preamble-2a515c.pch
-rw-r--r-- 1 aaron users 61885536 Okt  9 22:23 /tmp/preamble-2baca3.pch
-rw-r--r-- 1 aaron users 76913424 Okt  9 22:23 /tmp/preamble-9853df.pch
-rw-r--r-- 1 aaron users 64254872 Okt  9 22:23 /tmp/preamble-cbb035.pch
-rw-r--r-- 1 aaron users 64829792 Okt  9 22:24 /tmp/preamble-dd9bf1.pch
aaron:~/src/llvm-project> du -s /tmp/ 2>/dev/null
391204  /tmp/

Now I actually have /tmp as tmpfs, and so these 400 MB don't come for free. This is perfectly fine when I'm actually working on these files, but so far I have only opened KDevelop.

I see. The problem really is that the preamble files shouldn't be written to /tmp then - it would be perfectly fine to move them to a temp dir on the disk I think - it would still be faster than having to redo the work.

In D18551#412611, @mwolff wrote:

In D18551#410223, @aaronpuchert wrote:

In D18551#409551, @mwolff wrote:

When we didn't create the preamble, we will have to reparse the file completely. And this can easily take 1-2s per file, which is *really* annoying.

It only happens on the very first edit though, and will only be noticeable if code completion is required immediately after starting to edit the file. All following parses and code completions can use the precompiled preamble then. I don't think that's too bad. It's also not unusual, many applications have something like a "warmup phase" where not everything is loaded yet.

Yes, but hiding or minimizing the warmup phase is good. And it would be possible by having the flag active for opened files like I request.

Creating a preamble is actually more expensive than not creating it. So we make the initial parsing slower. This is only faster for files are actually edited.

Yes, that's why I definitely agree that we should not do it for files that aren't opened in an editor.

Creating it immediately only improves the reaction time for the first edit. For that small benefit, we're paying a hefty price.

Yes, I agree that in your case the price is high and the benefit small. In my case it usually isn't that big a deal, but YMMV :) And I'm all for improving this state somehow.

Also note how we just reparsed the file solely for code completion purposes!

You mean the preamble? Since code completion is usually invoked after some initial changes, we have to reparse anyway. What you said would only be true if we invoke code completion without changing anything first, which seems rather weird.

I mean: We trigger a reparse when we open a document, to get hold of the clang TU to do code completion. If this *does not* create a preamble, then the following call to query code completion items - *after* editing - will create the preamble. The preamble only includes the headers of the file, not the contents of the .cpp.

Well, then why don't we just skip the initial reparse, and only reparse after the first edit?

Yes, I agree that we should try this out. Note that this will require changes in clang/codecompletion/model.cpp at least, potentially also in other places. I.e. right now this code relies on the fact that we trigger a reparse after opening a document. This then allows us to write code like this there:

auto top = DUChainUtils::standardContextForUrl(m_url);
if (!top) {
    qCWarning(KDEV_CLANG) << "No context found for" << m_url;
    return;
}

ParseSessionData::Ptr sessionData(ClangIntegration::DUChainUtils::findParseSessionData(top->url(), m_index->translationUnitForUrl(top->url())));

if (!sessionData) {
    // TODO: trigger reparse and re-request code completion
    qCWarning(KDEV_CLANG) << "No parse session / AST attached to context for url" << m_url;
    return;
}

If we only attach the AST after the first edit, it won't be enough. We will also have to trigger a reparse and force attaching the AST/parse session data when code completion is explicitly requested via ctrl + space without editing.

You usually edit the .cpp file and then you want to get code completion results immediately.

We're talking about a one-time delay, all further edits will have a preamble.

Yes, but the patch proposed here in its current form just disables the preamble always, no? So I think we agree that this isn't good enough?

Having a preamble is crucial for good performance here.

I'm not suggesting to not create a preamble at all, but to create it only when we know it is needed.

OK, then I think we are on the same page - just this patch needs more love :)

To keep this short I didn't reply to the parts I completely agree with.

In D18551#604249, @mwolff wrote:

Now I actually have /tmp as tmpfs, and so these 400 MB don't come for free. This is perfectly fine when I'm actually working on these files, but so far I have only opened KDevelop.

I see. The problem really is that the preamble files shouldn't be written to /tmp then - it would be perfectly fine to move them to a temp dir on the disk I think - it would still be faster than having to redo the work.

There are arguments for both in-memory and on-disk, it depends on how much memory there is. I have another more more generously sized machine where the files are perfectly fine in RAM.

I think /tmp is correct by the FHS and /var/tmp wouldn't be appropriate. I could easily put this on disk if I wanted to. I just wanted to make the argument that resource usage is a concern, and that there is some benefit in limiting ourselves to the preambles that we need.

Also note how we just reparsed the file solely for code completion purposes!

You mean the preamble? Since code completion is usually invoked after some initial changes, we have to reparse anyway. What you said would only be true if we invoke code completion without changing anything first, which seems rather weird.

I mean: We trigger a reparse when we open a document, to get hold of the clang TU to do code completion. If this *does not* create a preamble, then the following call to query code completion items - *after* editing - will create the preamble. The preamble only includes the headers of the file, not the contents of the .cpp.

Well, then why don't we just skip the initial reparse, and only reparse after the first edit?

Yes, I agree that we should try this out. Note that this will require changes in clang/codecompletion/model.cpp at least, potentially also in other places. I.e. right now this code relies on the fact that we trigger a reparse after opening a document. This then allows us to write code like this there:

auto top = DUChainUtils::standardContextForUrl(m_url);
if (!top) {
    qCWarning(KDEV_CLANG) << "No context found for" << m_url;
    return;
}
 
ParseSessionData::Ptr sessionData(ClangIntegration::DUChainUtils::findParseSessionData(top->url(), m_index->translationUnitForUrl(top->url())));
 
if (!sessionData) {
    // TODO: trigger reparse and re-request code completion
    qCWarning(KDEV_CLANG) << "No parse session / AST attached to context for url" << m_url;
    return;
}

If we only attach the AST after the first edit, it won't be enough. We will also have to trigger a reparse and force attaching the AST/parse session data when code completion is explicitly requested via ctrl + space without editing.

How bad would it be if we don't do that and only provide KTextEditor-completions before the first edit? I think it's pretty rare that code completion is immediately requested, typically one starts with a line break, or by removing code, before new code is written.

You usually edit the .cpp file and then you want to get code completion results immediately.

We're talking about a one-time delay, all further edits will have a preamble.

Yes, but the patch proposed here in its current form just disables the preamble always, no? So I think we agree that this isn't good enough?

It shouldn't, unless I'm misunderstanding our usage of libclang. Without the flag, the preamble is created not on the first parse, but on the first reparse of a file. Meaning there is no preamble when I just open a file, but as soon as I edit and the IDE triggers a reparse, the preamble is created and used for subsequent reparsing.

Let's do some math to clarify this. Let

• C_all be the cost for parsing a source without preamble,
• C_source be the cost for parsing a source with preamble, and
• C_preamble be the cost for serializing the preamble.

Obviously C_source < C_all. Then we have:

Now let m_read be the number of documents that are only being read and m_write the number of documents being read and written. Then the cost difference is m_write · (C_all - C_source) - m_read · C_preamble, which is negative iff m_write / m_read < C_preamble / (C_all - C_source). The right-hand side is a constant that depends on the nature of the project and the hardware one is working on:

• C_all and C_source include reading from disk or page cache and doing some heavy CPU work, the difference between them is the parsing of the header files,
• C_preamble includes a bit of CPU work and writing to a file (which might be on disk).

The left-hand side depends on usage patterns. I use KDevelop a lot to read and browse code without doing any changes, so this would very likely benefit me. But I might be alone in that.

Edit: replaced time in calculation by cost.

In D18551#604847, @aaronpuchert wrote:

To keep this short I didn't reply to the parts I completely agree with.

In D18551#604249, @mwolff wrote:

Now I actually have /tmp as tmpfs, and so these 400 MB don't come for free. This is perfectly fine when I'm actually working on these files, but so far I have only opened KDevelop.

I see. The problem really is that the preamble files shouldn't be written to /tmp then - it would be perfectly fine to move them to a temp dir on the disk I think - it would still be faster than having to redo the work.

There are arguments for both in-memory and on-disk, it depends on how much memory there is. I have another more more generously sized machine where the files are perfectly fine in RAM.

I think /tmp is correct by the FHS and /var/tmp wouldn't be appropriate. I could easily put this on disk if I wanted to. I just wanted to make the argument that resource usage is a concern, and that there is some benefit in limiting ourselves to the preambles that we need.

Yes, I agree. Let's leave it at /tmp and try to reduce the amount of PCHs we put there (see below).

Also note how we just reparsed the file solely for code completion purposes!

You mean the preamble? Since code completion is usually invoked after some initial changes, we have to reparse anyway. What you said would only be true if we invoke code completion without changing anything first, which seems rather weird.

I mean: We trigger a reparse when we open a document, to get hold of the clang TU to do code completion. If this *does not* create a preamble, then the following call to query code completion items - *after* editing - will create the preamble. The preamble only includes the headers of the file, not the contents of the .cpp.

Well, then why don't we just skip the initial reparse, and only reparse after the first edit?

Yes, I agree that we should try this out. Note that this will require changes in clang/codecompletion/model.cpp at least, potentially also in other places. I.e. right now this code relies on the fact that we trigger a reparse after opening a document. This then allows us to write code like this there:

auto top = DUChainUtils::standardContextForUrl(m_url);
if (!top) {
    qCWarning(KDEV_CLANG) << "No context found for" << m_url;
    return;
}
 
ParseSessionData::Ptr sessionData(ClangIntegration::DUChainUtils::findParseSessionData(top->url(), m_index->translationUnitForUrl(top->url())));
 
if (!sessionData) {
    // TODO: trigger reparse and re-request code completion
    qCWarning(KDEV_CLANG) << "No parse session / AST attached to context for url" << m_url;
    return;
}

If we only attach the AST after the first edit, it won't be enough. We will also have to trigger a reparse and force attaching the AST/parse session data when code completion is explicitly requested via ctrl + space without editing.

How bad would it be if we don't do that and only provide KTextEditor-completions before the first edit? I think it's pretty rare that code completion is immediately requested, typically one starts with a line break, or by removing code, before new code is written.

Very bad. I can still remember how bad it felt to use KDevelop before @brauch fixed some nasty issues that prevented us from using the PCH properly. It always felt like KDevelop was utterly broken since it took so long to give us completion answers. And I believe I also remember that quite a few of our users complained about it, and in turn praised the improvement after @brauch fixed it.

I really think this is super important to have a snappy code completion behavior.

You usually edit the .cpp file and then you want to get code completion results immediately.

We're talking about a one-time delay, all further edits will have a preamble.

Yes, but the patch proposed here in its current form just disables the preamble always, no? So I think we agree that this isn't good enough?

It shouldn't, unless I'm misunderstanding our usage of libclang. Without the flag, the preamble is created not on the first parse, but on the first reparse of a file. Meaning there is no preamble when I just open a file, but as soon as I edit and the IDE triggers a reparse, the preamble is created and used for subsequent reparsing.

True, you are right on that point.

Let's do some math to clarify this. Let

• C_all be the cost for parsing a source without preamble,
• C_source be the cost for parsing a source with preamble, and
• C_preamble be the cost for serializing the preamble. Obviously C_source < C_all. Then we have:
  
  | | Cost for opening | Cost after first reparse | Cost after n-th reparse | | Old | C_all + C_preamble | C_all + C_source + C_preamble | C_all + n · C_source + C_preamble | | New | C_all | 2 · C_all + C_preamble | 2 · C_all + (n-1) · C_source + C_preamble | | New - Old | -C_preamble | C_all - C_source | C_all - C_source |
  
  Now let m_read be the number of documents that are only being read and m_write the number of documents being read and written. Then the cost difference is m_write · (C_all - C_source) - m_read · C_preamble, which is negative iff m_write / m_read < C_preamble / (C_all - C_source). The right-hand side is a constant that depends on the nature of the project and the hardware one is working on:
• C_all and C_source include reading from disk or page cache and doing some heavy CPU work, the difference between them is the parsing of the header files,
• C_preamble includes a bit of CPU work and writing to a file (which might be on disk).
  
  The left-hand side depends on usage patterns. I use KDevelop a lot to read and browse code without doing any changes, so this would very likely benefit me. But I might be alone in that.
  
  Edit: replaced time in calculation by cost.

:) I appreciate the thought put into this. And I totally agree on your analysis, and that for your usage pattern the proposal makes a lot of sense. But I and many others write a lot of code with KDevelop. And we tend to work in a completion-guided manner, i.e. I rarely write working code, I write some gibberish and KDevelop's auto completion inserts the right thing for me. If that suddenly takes significantly longer on the first time I start editing in a file, it means that KDevelop will look much less attractive. It may sound strange, but I assure you that this is a real problem. Even today we have many (valid, imo) concerns that code completion isn't fast enough. And this is pretty much the most important feature of KDevelop for me...

So that's why I once again suggest we only attach the preamble PCH to files we open. This should already remove the cost for C_preamble from a lot of files, since the majority of a project won't be opened in the editor. To me this looks like an advantage to all of us, so that would be cool if you could implement that!

Then, to make your use-case happy, I wouldn't mind adding a "hidden" feature that disables the PCH-on-first-parse for opened documents, based on an env var. That would allow us to test the impact of this feature and get a concrete feeling for it easily.

Secondly, once that's in, I suggest we work on another feature that I've long wanted to write, but never got around doing: A LRU cache of TUs. I.e. instead of keeping the TU for all opened documents in memory, and thereby also keeping the preamble PCH alive, only do that for a user-defined number of documents based on last usage. I would really like to see this, but we also will have to ensure that navigating quickly through all documents doesn't trigger excessive reparsing to add the TUs (think CTRL + Tab or alt+arrow)... Definitely another interesting area to investigate, but also harder to get right probably.

You don't have control over where those pch files are written, right, other than through setting $TMPDIR?

Because it seems that if that control existed the best solution might be to store the pchs corresponding to the open files in a session in a session-specific cache directory (and trash the ones of files no longer open when you close the session).

In D18551#605577, @mwolff wrote:

How bad would it be if we don't do that and only provide KTextEditor-completions before the first edit? I think it's pretty rare that code completion is immediately requested, typically one starts with a line break, or by removing code, before new code is written.

Very bad. I can still remember how bad it felt to use KDevelop before @brauch fixed some nasty issues that prevented us from using the PCH properly. It always felt like KDevelop was utterly broken since it took so long to give us completion answers. And I believe I also remember that quite a few of our users complained about it, and in turn praised the improvement after @brauch fixed it.

You're referring to D6905? That contains a lot of other things. I don't think that adding the flag was the core part of that change, and @brauch doesn't think so either:

In D18551#402186, @brauch wrote:

To clarify, the patch back then was less about the first-time delay, and more about the delay being there *every* time because of wrong usage of the API. Whether the delay occurs once is arguably a bit less important.

Clearly that was a pretty nasty bug, and it's obvious people were noticing that.

:) I appreciate the thought put into this. And I totally agree on your analysis, and that for your usage pattern the proposal makes a lot of sense. But I and many others write a lot of code with KDevelop.

Just to clarify this: I also write code, although unfortunately not so much for KDE at the moment.

However, I work on projects which much larger source files, and I know how long the code completion takes sometimes. Still, I'm hardly bothered by a one-time delay if that saves me resources that I can invest elsewhere.

If [code completion] suddenly takes significantly longer on the first time I start editing in a file, it means that KDevelop will look much less attractive. It may sound strange, but I assure you that this is a real problem.

Every program, including the operating system kernel, has warmup behavior due to caching. When KDevelop starts for the first time, most of the libraries it needs will likely not be in the page cache. This is quite common behavior, and for a good reason: doing things “lazy” makes sure you don't do more than needed. For that reason it is also the default behavior of the clang-c/Index.h API.

Now about our situation: You have to wait for the initial parse anyway. The initial parse should actually be faster with this change, because we save the cost of serializing the preamble. After that, all the files are at least in the page cache, so the second parse should already be faster.

Even today we have many (valid, imo) concerns that code completion isn't fast enough.

One of the problems might be that many templates from headers are only instantiated in the source file, and these instantiations aren't contained in the preamble. But that is an issue for Clang upstream.

So that's why I once again suggest we only attach the preamble PCH to files we open. This should already remove the cost for C_preamble from a lot of files, since the majority of a project won't be opened in the editor. To me this looks like an advantage to all of us, so that would be cool if you could implement that!

I understand your proposal, and I'll go down that route if we can't agree on this change.

Then, to make your use-case happy, I wouldn't mind adding a "hidden" feature that disables the PCH-on-first-parse for opened documents, based on an env var. That would allow us to test the impact of this feature and get a concrete feeling for it easily.

I don't think I have such a special use case. Do people not read code? That would explain the quality of some projects, for sure. But I don't believe it. Obviously neither of us has any data to back up our claims, so we have to stick to speculation here.

Secondly, once that's in, I suggest we work on another feature that I've long wanted to write, but never got around doing: A LRU cache of TUs. I.e. instead of keeping the TU for all opened documents in memory, and thereby also keeping the preamble PCH alive, only do that for a user-defined number of documents based on last usage. I would really like to see this, but we also will have to ensure that navigating quickly through all documents doesn't trigger excessive reparsing to add the TUs (think CTRL + Tab or alt+arrow)... Definitely another interesting area to investigate, but also harder to get right probably.

Agreed, this would be interesting, but caches have an enormously huge design space.

Hmm, now I'm thinking: If we parse first without creating a preamble, we still have the Clang AST. Couldn't we create the preamble from the AST without reparsing and use that for subsequent parsing?

I'll need to have a look at the API and implementation, maybe I'm missing something.

Re: the time and resources spent by the background parser when you'd rather not:

Didn't earlier versions (< 5.4 at least) add background parser jobs to the job tracker thingy that allows to stop ongoing activity via the 2 buttons (selective & "all") in the taskbar? I seem to recall that I could cancel at least the pending parser jobs which sometimes comes in quite handy.

Maybe that job tracker and its buttons should be moved out of the build and into an overall context, so they can be used to cancel any kind of background of activity that can be cancelled (project loading jobs also show up in there already)?

Rather than adding heuristics to the current code that aims for a compromise between apparently irreconcilable requirements it'd allow the user to intervene to prevent unjustifiable resource waste. And if after such an intervention s/he has to wait a little bit longer for parsing information to become available, well, at least they'd know why, right? NB: this wouldn't change anything for users who never even look at the job buttons.

Rene, I'm missing context - what are you replying to? This doesn't seem to be related to the preamble?

Rene, I'm missing context - what are you replying to? This doesn't seem to be related to the preamble?

Well, yes and no. If I'm not mistaken the goal in this PR is to limit resource "waste" by not generating lots of preambles unconditionally, whether they're to be used at once or not. Until now there have been proposed code changes that try to do this automatically, but I suppose you can also let the user intervene and cancel the jobs in charge of generating those preambles, i.e. the background parser jobs.

There must be times anyone would like to be able to stop the full-project-parse-on-load even if they have oodles of disk and RAM, because it still can take a lot of time and CPU you'd prefer to spend on something else (think loading a huge session/project to debug a possibly already running application).

I don't want to cancel any parse jobs, I want to change whether and when they emit the preamble.

Surely job control is also an interesting topic, one that I might be interested to join discussing, but I don't think it's related to this.

As I said, I thought the possibility to cancel jobs could be a comprise (I don't get the impression you've been able to find one yet), but OK. I've started to peek around in the code to see about job control, if these are based on KJob it shouldn't be hard to add them to the existing job controller.

In D18551#623098, @rjvbb wrote:

As I said, I thought the possibility to cancel jobs could be a comprise (I don't get the impression you've been able to find one yet), but OK.

To what end though? We are in full agreement that we don't want preambles for source files that aren't open.

Our discussion is about when to create the preamble for open documents. If I were to stop that job, I wouldn't get the AST, but I want the AST. I just don't want the preamble before I've started editing the file.

Our discussion is about when to create the preamble for open documents. If I were to stop that job, I wouldn't get the AST, but I want the AST. I just don't want the preamble before I've started editing the file.

OK, so I misread or misremembered. Sorry about that.

I've started looking around and making some preparatory changes to the class that tracks project parse jobs; I'll see if I can come up with something that's worth discussing. Don't hold your breath, my progress will be ... patchy at best :)

https://invent.kde.org/kde/kdevelop/-/merge_requests/114

I've now pushed another patch which will suspend TUs to reduce memory consumption further.

Where?

https://invent.kde.org/kde/kdevelop/-/commit/66253c6df81fb29f60df5f88e30dc29f57cdb36e