D18551: clang: Create preamble only on second parse

Tue Feb 4 02:41:01 GMT 2020

aaronpuchert added a comment.

  In D18551#605577 <https://phabricator.kde.org/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 <https://phabricator.kde.org/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 <https://phabricator.kde.org/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.

  Obviously 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.

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D18551

To: aaronpuchert, #kdevelop, mwolff, brauch, rjvbb
Cc: rjvbb, kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20200204/c5066c33/attachment-0001.html>