<div dir="ltr">On Wed, Aug 28, 2013 at 4:16 AM, Aleix Pol <span dir="ltr"><<a href="mailto:aleixpol@kde.org" target="_blank">aleixpol@kde.org</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">
<div dir="ltr"><div class="im">On Tue, Aug 27, 2013 at 10:48 AM, Milian Wolff <span dir="ltr"><<a href="mailto:mail@milianw.de" target="_blank">mail@milianw.de</a>></span> wrote:<br></div><div><div class="h5"><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"><div>On Tuesday 27 August 2013 02:10:36 Aleix Pol wrote:<br>
> I haven't had problems with it so far, I'll merge it later this week. I'm<br>
> not doing it yet because I just merged the noAreaTabs branch and I'd prefer<br>
> to break systems one at a time.<br>
><br>
> In any case, please test :)<br>
<br>
</div>I've successfully loaded my big kdev* session with 10 cmake projects in it. So<br>
it works - cool!<br>
<br>
Switching branches in kdevplatform from master to 1.4 triggers a crash though:<br>
Can you reproduce this?<br>
<br>
Thread 1 (Thread 0x7fc905a927c0 (LWP 17518)):<br>
[KCrash Handler]<br>
#5 0x00007fc86938b978 in CMakeManager::isReloading (this=0x30131b0, p=0x0) at<br>
/ssd/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:649<br>
#6 0x00007fc86938be10 in CMakeManager::realDirectoryChanged (this=0x30131b0,<br>
dir=...) at<br>
/ssd/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:685<br>
#7 0x00007fc86938bcf7 in CMakeManager::filesystemBuffererTimeout<br>
(this=0x30131b0) at<br>
/ssd/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:676<br>
#8 0x00007fc86937b83c in CMakeManager::qt_static_metacall (_o=0x30131b0,<br>
_c=QMetaObject::InvokeMetaMethod, _id=6, _a=0x7fffad95a410) at<br>
/ssd/milian/projects/.build/kde4/kdevelop/projectmanagers/cmake/moc_cmakemanager.cpp:71<br>
#9 0x00007fc916068ba8 in QMetaObject::activate(QObject*, QMetaObject const*,<br>
int, void**) () from /usr/lib/libQtCore.so.4<br>
#10 0x00007fc91606cd71 in QObject::event(QEvent*) () from<br>
/usr/lib/libQtCore.so.4<br>
#11 0x00007fc9151db12c in QApplicationPrivate::notify_helper(QObject*,<br>
QEvent*) () from /usr/lib/libQtGui.so.4<br>
#12 0x00007fc9151e16f0 in QApplication::notify(QObject*, QEvent*) () from<br>
/usr/lib/libQtGui.so.4<br>
#13 0x00007fc91687472a in KApplication::notify(QObject*, QEvent*) () from<br>
/usr/lib/libkdeui.so.5<br>
#14 0x00007fc916054efd in QCoreApplication::notifyInternal(QObject*, QEvent*)<br>
() from /usr/lib/libQtCore.so.4<br>
#15 0x00007fc916084633 in ?? () from /usr/lib/libQtCore.so.4<br>
#16 0x00007fc916081989 in ?? () from /usr/lib/libQtCore.so.4<br>
#17 0x00007fc90f11dd96 in g_main_context_dispatch () from<br>
/usr/lib/libglib-2.0.so.0<br>
#18 0x00007fc90f11e0e8 in ?? () from /usr/lib/libglib-2.0.so.0<br>
#19 0x00007fc90f11e18c in g_main_context_iteration () from<br>
/usr/lib/libglib-2.0.so.0<br>
#20 0x00007fc916081b85 in<br>
QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()<br>
from /usr/lib/libQtCore.so.4<br>
#21 0x00007fc9152782e6 in ?? () from /usr/lib/libQtGui.so.4<br>
#22 0x00007fc916053b5f in<br>
QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from<br>
/usr/lib/libQtCore.so.4<br>
#23 0x00007fc916053e55 in<br>
QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from<br>
/usr/lib/libQtCore.so.4<br>
#24 0x00007fc916058f8b in QCoreApplication::exec() () from<br>
/usr/lib/libQtCore.so.4<br>
#25 0x00000000004127db in main (argc=3, argv=0x7fffad95c998) at<br>
/ssd/milian/projects/kde4/kdevelop/app/main.cpp:561<br>
<br>
Furthermore looking at the code I have some things to note:<br>
<br>
a) Don't use QList in new code. <a href="http://marcmutz.wordpress.com/effective-qt/containers/" target="_blank">http://marcmutz.wordpress.com/effective-qt/containers/</a> Instead use QVector and mark custom structs as Q_MOVABLE_TYPE.<br>
<br>
b) Don't use QMap if you don't need sorting (which you probably don't). Use<br>
QHash. <a href="http://woboq.com/blog/qmap_qhash_benchmark.html" target="_blank">http://woboq.com/blog/qmap_qhash_benchmark.html</a> Yes, for small sets<br>
QMap may be faster but then operations are really fast anyways on small scale.<br>
<br>
c) Remove trailing spaces - come on Aleix ;-) Do me that favor pretty please?<br>
You can just enable the "remove trailing spaces on edited lines" feature in<br>
Kate - it's perfect!<br>
<br>
d) Some other style nitpicks like spaces around operators sometimes missing<br>
and placement of * or & not always following the style we use in other places<br>
(i.e. attach it to the type name).<br>
<br>
But don't get me wrong - the code looks really good all in all! The above is<br>
just nitpicks, which you could also ignore and I'll refactor eventually. Take<br>
it more as an educative post-review :)<br>
<br>
Many many thanks for working on this! /me is so happy that "grep -R QMutex"<br>
returns zero matches in the cmake folder now :) You are my hero Aleix!<br>
<div><div><br>
> On Tue, Aug 20, 2013 at 2:56 AM, Aleix Pol <<a href="mailto:aleixpol@kde.org" target="_blank">aleixpol@kde.org</a>> wrote:<br>
> > Hi!<br>
> > It's not news that we've had some threading issues with the project model,<br>
> > especially from the cmake plugin. This was mostly because the project was<br>
> > parsed in a different thread and the results where being added from that<br>
> > thread to the ProjectsModel. It worked, but killing a bug made another one<br>
> > to appear, so that was no fun.<br>
> ><br>
> > With this change, I'm making it so the project will be parsed from a<br>
> > separate thread and will pass the data to another thread that will do the<br>
> > tree creation.<br>
> ><br>
> > I haven't tested it (at all, despite small tests with test projects) but<br>
> > the unit tests pass and I'm feeling quite positive about it. I'll be<br>
> > testing it in my system making sure all's in order during the following<br>
> > weeks, but for now, if somebody is curious enough, I'm interested in any<br>
> > kind of feedback.<br>
> ><br>
> > Cheers!<br>
> > Aleix<br>
> ><br>
> > PS: I'm applying this patch in my kdevplatform, for further safety ;)<br>
> > <a href="http://paste.opensuse.org/63054558" target="_blank">http://paste.opensuse.org/63054558</a><br>
<br>
</div></div><span><font color="#888888">--<br>
Milian Wolff<br>
<a href="mailto:mail@milianw.de" target="_blank">mail@milianw.de</a><br>
<a href="http://milianw.de" target="_blank">http://milianw.de</a><br>
</font></span></blockquote></div><br></div></div></div><div class="gmail_extra">Fixed a) and b), and a couple of more issues I found, still haven't been able to reproduce your <span style="font-family:arial,sans-serif;font-size:13px">ASSERT: "ctx".</span></div>
<span class=""><font color="#888888">
<div class="gmail_extra"><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div class="gmail_extra"><span style="font-family:arial,sans-serif;font-size:13px">Aleix</span></div></font></span></div>
</blockquote></div><br></div><div class="gmail_extra">This reloading problem should be fixed now, if no further bugs appear, I'll merge next monday.<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">
Aleix</div></div>