<div dir="ltr"><div>I have a thought, maybe completely off-base since I haven't actually worked on this (hard!) problem.<br><br>I think there are two issues
masquerading as one, and it's easier to see if you think in terms of
"enviroments" rather than "contexts":<br>1. This environment is
/outdated/ (user added a pch, added new defines, etc). What we want is clear:
we want all TopDUContexts using it to be rebuilt<br>2. This environment
is /different/ from another environment, both environments may be valid
at the same time for the same TopDUContext. The solution is more
challenging.<br><br></div>I get the feeling that your "Update DUChain data when the environment has changed" was targeting #1, and then we stumbled into #2.<br><div><br>Now, two assumptions:<br>1. We don't want ping-pong: Given context X, included in context Y and created with env A, X should never be recreated while A (or at least Y?) remains valid. <br>
2. We don't want multiple context bloat: When context Z later includes X with env B, X remains unchanged.<br><br></div><div>Given these assumptions, now two complementary solutions:<br></div><div>1. We need to be able to identify a stale environment and rebuild dependant contexts. Can IADM help here? Not sure how hard this would be.<br>
</div><div>2. At some point in the future, I'd like to be able to click on a #include and select "rebuild from this environment". This would be my answer to problem and assumption #2.<br></div><div><br></div>
<div>For the time being, we at least shouldn't be worse off if we could just get #1... but is that a realistic solution or is it deceptively complicated (or wrong)?</div><div><br></div><div>Thoughts?<br><br></div><div>
-Olivier JG<br></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Aug 30, 2014 at 12:25 AM, Milian Wolff <span dir="ltr"><<a href="mailto:mail@milianw.de" target="_blank">mail@milianw.de</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hey all,<br>
<br>
Sergey noticed a big performance slow-down that I introduced along with some<br>
more bugs with this commit:<br>
<br>
commit 5215ff8f78ba19bd5a3b8264b7bbe9449532b03f<br>
Author: Milian Wolff <<a href="mailto:mail@milianw.de">mail@milianw.de</a>><br>
Date: Thu Aug 7 19:10:54 2014 +0200<br>
<br>
Update DUChain data when the environment has changed.<br>
<br>
This combines the include paths, defines and pch-path into a<br>
hash which is stored on-disk and then later compared to the new<br>
environment. If the hash differs, we trigger a reparse.<br>
<br>
To prevent opened files from getting reparsed at startup when<br>
no data from the project could be obtained, we add some more code<br>
for this special purpose: We check whether we parsed before<br>
with a known project and whether the new environment data also<br>
comes from a project. If not then we rely only on the timestamp<br>
of the file on whether to trigger a reparse or not. Otherwise, the<br>
previous data (i.e. with known project) takes precedence.<br>
<br>
For some more input, if you didn't read this already, see:<br>
<a href="https://git.reviewboard.kde.org/r/119959/" target="_blank">https://git.reviewboard.kde.org/r/119959/</a><br>
<br>
Now I looked at the remaining issues that Sergey noticed and can confirm them.<br>
A simple project to reproduce this can be created like this:<br>
<br>
~~~~~~~ CMakeLists.txt: ~~~~~~~~~~<br>
cmake_minimum_required(VERSION 2.8.11)<br>
project(test)<br>
<br>
add_executable(fileA fileA.cpp)<br>
set_property(TARGET fileA APPEND PROPERTY INCLUDE_DIRECTORIES "/tmp/foo")<br>
<br>
add_executable(fileB fileB.cpp)<br>
set_property(TARGET fileB APPEND PROPERTY INCLUDE_DIRECTORIES "/tmp/bar")<br>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~<br>
<br>
And both fileA.cpp and fileB.cpp just need to have something like this:<br>
<br>
~~~~~~~~~~~~~~~~~~<br>
#include <iostream><br>
int foo() {}<br>
~~~~~~~~~~~~~~~~~~<br>
<br>
Now run this in a kdev-clang KDevelop session and enable the corresponding<br>
debug area. Once both files are cached/highlighted, change one of them, wait<br>
for the update, then the other. With change I mean e.g. add an argument to the<br>
function or anything like that, but keep the iostream include.<br>
<br>
What you'll see is tons of output like this:<br>
environment differs, require update: "/usr/include/c++/4.9.1/iostream" new<br>
hash: 4014444178 new project known: true old hash: 173631101 old project<br>
known: true<br>
<br>
This is *valid*, since both files have different include paths. This, and the<br>
per-file defines get inherited by included files (oh C++ modules, where are<br>
you?). As such, from a compiler perspective, it's correct that the cache needs<br>
to be updated once the environment has changed.<br>
<br>
>From an IDE perspective this is unbearably slow, I agree with Sergey. But what<br>
should we do about this situation? I have three suggestions so far:<br>
<br>
#1 skip update of duchain cache for system includes on environment changes<br>
+ relatively easy to implement thanks to clang_Location_isInSystemHeader<br>
- the cache ping-pong will still happen for non-system-includes though<br>
note: the cache will still be updated when the timestamp of the file changes<br>
note: forced recursive reparses will also still trigger an update<br>
=> I think I'll add this as a first work-around.<br>
<br>
#2 combine Sergeys idea with my existing environment checking<br>
Sergey tried to fix the problem by changing the environment that is serialized<br>
to only reference the include paths that where actually used by a given file.<br>
This breaks the update mechanism though. But what one could do is store two<br>
hashes, one for the parse job to check whether a clang reparse is required,<br>
and one to check whether the duchain cache needs an updated.<br>
- a bit more involved to implement<br>
+ should hopefully also guard against the cache ping-pong for non-system-<br>
includes, as long as you don't do funny includes of different files based on<br>
the include path<br>
- completely ignores the macro defines though which must be handled similarly<br>
otherwise you can get the same cache ping-pong effects. to also figure out<br>
what defines of the environment where used, we'd have to iterate over all<br>
cursors and find macro uses and check that against the ones in the<br>
environment, which is probably quite costly...<br>
<br>
#3 do what oldcpp did<br>
- I still have to understand what exactly it is doing<br>
- I /think/ it's something like #2, but it also creates multiple cache entries<br>
per file, depending on the environment. this blows up the size of the duchain<br>
cache and the memory usage etc. pp. I'm not sure we want this<br>
<br>
Any other suggestions how to handle this?<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Milian Wolff<br>
<a href="mailto:mail@milianw.de">mail@milianw.de</a><br>
<a href="http://milianw.de" target="_blank">http://milianw.de</a><br>
_______________________________________________<br>
KDevelop-devel mailing list<br>
<a href="mailto:KDevelop-devel@kde.org">KDevelop-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/kdevelop-devel" target="_blank">https://mail.kde.org/mailman/listinfo/kdevelop-devel</a><br>
</font></span></blockquote></div><br></div>