Refactoring CMake support

Tue Aug 27 08:48:20 UTC 2013

On Tuesday 27 August 2013 02:10:36 Aleix Pol wrote:
> I haven't had problems with it so far, I'll merge it later this week. I'm
> not doing it yet because I just merged the noAreaTabs branch and I'd prefer
> to break systems one at a time.
> 
> In any case, please test :)

I've successfully loaded my big kdev* session with 10 cmake projects in it. So 
it works - cool!

Switching branches in kdevplatform from master to 1.4 triggers a crash though:
Can you reproduce this?

Thread 1 (Thread 0x7fc905a927c0 (LWP 17518)):
[KCrash Handler]
#5  0x00007fc86938b978 in CMakeManager::isReloading (this=0x30131b0, p=0x0) at 
/ssd/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:649
#6  0x00007fc86938be10 in CMakeManager::realDirectoryChanged (this=0x30131b0, 
dir=...) at 
/ssd/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:685
#7  0x00007fc86938bcf7 in CMakeManager::filesystemBuffererTimeout 
(this=0x30131b0) at 
/ssd/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:676
#8  0x00007fc86937b83c in CMakeManager::qt_static_metacall (_o=0x30131b0, 
_c=QMetaObject::InvokeMetaMethod, _id=6, _a=0x7fffad95a410) at 
/ssd/milian/projects/.build/kde4/kdevelop/projectmanagers/cmake/moc_cmakemanager.cpp:71
#9  0x00007fc916068ba8 in QMetaObject::activate(QObject*, QMetaObject const*, 
int, void**) () from /usr/lib/libQtCore.so.4
#10 0x00007fc91606cd71 in QObject::event(QEvent*) () from 
/usr/lib/libQtCore.so.4
#11 0x00007fc9151db12c in QApplicationPrivate::notify_helper(QObject*, 
QEvent*) () from /usr/lib/libQtGui.so.4
#12 0x00007fc9151e16f0 in QApplication::notify(QObject*, QEvent*) () from 
/usr/lib/libQtGui.so.4
#13 0x00007fc91687472a in KApplication::notify(QObject*, QEvent*) () from 
/usr/lib/libkdeui.so.5
#14 0x00007fc916054efd in QCoreApplication::notifyInternal(QObject*, QEvent*) 
() from /usr/lib/libQtCore.so.4
#15 0x00007fc916084633 in ?? () from /usr/lib/libQtCore.so.4
#16 0x00007fc916081989 in ?? () from /usr/lib/libQtCore.so.4
#17 0x00007fc90f11dd96 in g_main_context_dispatch () from 
/usr/lib/libglib-2.0.so.0
#18 0x00007fc90f11e0e8 in ?? () from /usr/lib/libglib-2.0.so.0
#19 0x00007fc90f11e18c in g_main_context_iteration () from 
/usr/lib/libglib-2.0.so.0
#20 0x00007fc916081b85 in 
QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () 
from /usr/lib/libQtCore.so.4
#21 0x00007fc9152782e6 in ?? () from /usr/lib/libQtGui.so.4
#22 0x00007fc916053b5f in 
QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from 
/usr/lib/libQtCore.so.4
#23 0x00007fc916053e55 in 
QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from 
/usr/lib/libQtCore.so.4
#24 0x00007fc916058f8b in QCoreApplication::exec() () from 
/usr/lib/libQtCore.so.4
#25 0x00000000004127db in main (argc=3, argv=0x7fffad95c998) at 
/ssd/milian/projects/kde4/kdevelop/app/main.cpp:561

Furthermore looking at the code I have some things to note:

a) Don't use QList in new code. http://marcmutz.wordpress.com/effective-qt/containers/ Instead use QVector and mark custom structs as Q_MOVABLE_TYPE.

b) Don't use QMap if you don't need sorting (which you probably don't). Use 
QHash. http://woboq.com/blog/qmap_qhash_benchmark.html Yes, for small sets 
QMap may be faster but then operations are really fast anyways on small scale.

c) Remove trailing spaces - come on Aleix ;-) Do me that favor pretty please? 
You can just enable the "remove trailing spaces on edited lines" feature in 
Kate - it's perfect!

d) Some other style nitpicks like spaces around operators sometimes missing 
and placement of * or & not always following the style we use in other places 
(i.e. attach it to the type name).

But don't get me wrong - the code looks really good all in all! The above is 
just nitpicks, which you could also ignore and I'll refactor eventually. Take 
it more as an educative post-review :)

Many many thanks for working on this! /me is so happy that "grep -R QMutex" 
returns zero matches in the cmake folder now :) You are my hero Aleix!

> On Tue, Aug 20, 2013 at 2:56 AM, Aleix Pol <aleixpol at kde.org> wrote:
> > Hi!
> > It's not news that we've had some threading issues with the project model,
> > especially from the cmake plugin. This was mostly because the project was
> > parsed in a different thread and the results where being added from that
> > thread to the ProjectsModel. It worked, but killing a bug made another one
> > to appear, so that was no fun.
> > 
> > With this change, I'm making it so the project will be parsed from a
> > separate thread and will pass the data to another thread that will do the
> > tree creation.
> > 
> > I haven't tested it (at all, despite small tests with test projects) but
> > the unit tests pass and I'm feeling quite positive about it. I'll be
> > testing it in my system making sure all's in order during the following
> > weeks, but for now, if somebody is curious enough, I'm interested in any
> > kind of feedback.
> > 
> > Cheers!
> > Aleix
> > 
> > PS: I'm applying this patch in my kdevplatform, for further safety ;)
> > http://paste.opensuse.org/63054558

-- 
Milian Wolff
mail at milianw.de
http://milianw.de