<div dir="ltr">On Wed, Jul 24, 2013 at 9:42 PM, Milian Wolff <span dir="ltr"><<a href="mailto:mail@milianw.de" target="_blank">mail@milianw.de</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

Hey all, esp. Aleix.<br>
<br>
While working on the project filter I encounter an issue when trying to<br>
integrate it into the CMake ProjectManager: Since the CMake manager uses a<br>
threaded import, I would need to make the project filters threadsafe which imo<br>
should not be done.<br>
<br>
Generally, considering the history of bugs and crashes as well as looking at<br>
the hacks and workarounds we have added, I think it's time to revisit the<br>
decision to "parallelize" the cmake import.<br>
<br>
I'm all for optimizations, as you all hopefully know, but this is simply bad.<br>
Esp. considering that for every crash that was encountered, another mutex was<br>
added (similar to the DUChain code) this does not even scale half as good as<br>
it should.<br>
<br>
So, I propose the following:<br>
<br>
# kill the threading<br>
<br>
- remove the QtConcurrent stuff from the ImportProjectJob<br>
- kill all mutices and locking in the CMakeProjectManager (except for DUChain<br>
stuff)<br>
- remove the project model threading hack<br>
<br>
# refactor the code<br>
<br>
- only support async "import" operations in ProjectFileManager, i.e. only<br>
"createImportJob" and get rid of "import" and "parse"<br>
- make the ImportProjectJob a KCompositeJob such that its possible to add<br>
child jobs to it<br>
- add new child jobs for every CMakeLists.txt (share code if possible, we need<br>
the same for Makefiles and QMake files in their respective managers)<br>
NOTE: these jobs then internally should parse the files in a separate thread<br>
- this should also make it trivial to use the AbstractFileManager code for the<br>
CMakeManager and only add the custom CMakeLists.txt parsing + target creation<br>
+ refactoring on top of that<br>
<br>
Note that with the KJob API in place, we should _never_ interact with the<br>
ProjectController or ProjectModel or any KSharedConfig from a background<br>
thread! The interpretation of e.g. a CMakeLists.txt should only give<br>
information about defines, include paths, targets etc. pp. The respective<br>
ProjectTargetItems + contents should also be created in the main thread!<br>
<br>
Furthermore note that the files of a given project get only added to the<br>
background parser after the complete Import has finished (which is good, we<br>
need includes, defines etc. pp.). This is what we have now, and what we need<br>
to keep. The above KCompositeJob API should handle it nicely, I think.<br>
<br>
# speed!<br>
<br>
Once the above is done, we need to write a proper benchmark for opening a<br>
session of X projects in the CMakeProjectManager. Then we can look into<br>
profiling it and optimizing it properly.<br>
<br>
# Profit<br>
<br>
The results should be cleaner code, less bugs and eventually maybe even better<br>
performance due to less lock contention.<br>
<br>
Who's up the task and wants to help? Of course, this should happen in a<br>
separate branch.<br>
<br>
Cheers<br>
<span class=""><font color="#888888">--<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><div class="gmail_extra">Yes, we can try to do something like that, we have separated that data in the past in the CMakeProjectData tuple (since David had plans to serialize it, which never happened).</div>

<div class="gmail_extra"><br></div><div class="gmail_extra">Anyway, if you give me some time, I can look into moving the threading into the plugin, which I agree that could be of some benefit.</div><div class="gmail_extra">

That said, note that it will require some fundamental refactoring of the import API.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Aleix</div></div>