Refactoring CMakeProjectManager - threading revisited

[Andreas Pakulat]

Hi,

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

> On Wednesday 24 July 2013 22:20:55 Andreas Pakulat wrote:
> > Hi,
> >
> > 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
> >
> > I'm wondering if the work could be split up into more/separate
> milestones,
> > i.e. as a first step just make the import-job not use threading anymore
> > (and block the UI during the import again). Then refactor it so it runs
> > multiple child-jobs until everything is done in batches so the UI gets
> > responsive again. Then you can drop all the mutexes from the cmake
> manager.
> > At that point you'd be thread-free, but probably quite a bit slower again
> > (and blocking the UI more), however the individual steps could be
> > easier/faster to accomplish and you don't live in a broken branch for
> such
> > a long time.
> >
> > Once you have the import job as composite and sub-jobs you could start
> > moving the sub-jobs to the manager so it can do its own threading if it
> > wants to.
>
> Yes, that's what I have in mind as well. The bullet points and sections I
> wrote above should all be more or less separated. I still want this work to
> happen in a separate branch though.
>

Oh, no doubt about that. Anything else would be insane :)


> But do you agree in principle with the idea and the steps I outlined above?
>

I cannot think of any extra steps necessary at least. Moving the threading
completely into the manager may require a tad bit more code because you
need structs to store the includes/defines/targets in the threads and
transport the infos to the ui thread code. Could be overall its still less
code of course.

The one thing I'm not sure will work out is separating the parsing per
CMakeLists.txt. These files are usually written in ways that require
parsing everything at once - or reparsing parent/sibling files again and
again.

Andreas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130725/a3a0e012/attachment.html>