<div dir="ltr">Hi,<div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jul 24, 2013 at 10:45 PM, 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"><div class="HOEnZb"><div class="h5">On Wednesday 24 July 2013 22:20:55 Andreas Pakulat wrote:<br>
> Hi,<br>
><br>
> On Wed, Jul 24, 2013 at 9:42 PM, Milian Wolff <<a href="mailto:mail@milianw.de">mail@milianw.de</a>> wrote:<br>
> > 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<br>
> > imo<br>
> > should not be done.<br>
> ><br>
> > Generally, considering the history of bugs and crashes as well as looking<br>
> > 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<br>
> > bad.<br>
> > Esp. considering that for every crash that was encountered, another mutex<br>
> > was<br>
> > added (similar to the DUChain code) this does not even scale half as good<br>
> > 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<br>
> > 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<br>
> > 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<br>
> > thread<br>
> > - this should also make it trivial to use the AbstractFileManager code for<br>
> > the<br>
> > CMakeManager and only add the custom CMakeLists.txt parsing + target<br>
> > creation<br>
> > + refactoring on top of that<br>
><br>
> I'm wondering if the work could be split up into more/separate milestones,<br>
> i.e. as a first step just make the import-job not use threading anymore<br>
> (and block the UI during the import again). Then refactor it so it runs<br>
> multiple child-jobs until everything is done in batches so the UI gets<br>
> responsive again. Then you can drop all the mutexes from the cmake manager.<br>
> At that point you'd be thread-free, but probably quite a bit slower again<br>
> (and blocking the UI more), however the individual steps could be<br>
> easier/faster to accomplish and you don't live in a broken branch for such<br>
> a long time.<br>
><br>
> Once you have the import job as composite and sub-jobs you could start<br>
> moving the sub-jobs to the manager so it can do its own threading if it<br>
> wants to.<br>
<br>
</div></div>Yes, that's what I have in mind as well. The bullet points and sections I<br>
wrote above should all be more or less separated. I still want this work to<br>
happen in a separate branch though.<br></blockquote><div><br></div><div style>Oh, no doubt about that. Anything else would be insane :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

But do you agree in principle with the idea and the steps I outlined above?<br></blockquote><div><br></div><div style>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.</div>
<div style><br></div><div style>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.</div>
<div style><br></div><div style>Andreas</div></div></div></div>