Refactoring CMakeProjectManager - threading revisited

[Aleix Pol]

On Wed, Jul 24, 2013 at 9:42 PM, Milian Wolff <mail at milianw.de> wrote:

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

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

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.
That said, note that it will require some fundamental refactoring of the
import API.

Aleix
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130729/2e00e20e/attachment.html>