D10124: Faster simplejob start

jtamate added a comment. In D10124#213918 , @dfaure wrote: > This commit leads to > > 20:29:06.184 okteta(12932) QObject::connect|?libKF5KIOCore.so.5? QObject::connect: No such slot KIO::ListJob::slotTotalSize(KIO::filesize_t) in /d/kd

D10124: Faster simplejob start

dfaure added a comment. This commit leads to 20:29:06.184 okteta(12932) QObject::connect|?libKF5KIOCore.so.5? QObject::connect: No such slot KIO::ListJob::slotTotalSize(KIO::filesize_t) in /d/kde/src/5/frameworks/kio/src/core/listjob.cpp:289 Please fix ;) REPOSITORY R241 KIO REV

D10124: Faster simplejob start

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R241:42fed2e70a21: Faster simplejob start (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https:/

D10124: Faster simplejob start

dfaure accepted this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks, mwolff, dfaure Cc: mwolff, broulik, ngraham, anthonyfieroni, michaelh

D10124: Faster simplejob start

jtamate updated this revision to Diff 26787. jtamate added a comment. Removed the private slots. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10124?vs=26239&id=26787 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10124 AFFECTED FILES src/

D10124: Faster simplejob start

dfaure added a comment. Looks good, but this means you can also remove the Q_PRIVATE_SLOT declaration for those slots defined in the private class (slotConnected, _k_foo etc.) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks, mwolff, dfaure

D10124: Faster simplejob start

jtamate added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks, mwolff, dfaure Cc: mwolff, broulik, ngraham, anthonyfieroni, michaelh

D10124: Faster simplejob start

broulik added a comment. So, good to go? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks, mwolff Cc: mwolff, broulik, ngraham, anthonyfieroni, michaelh

D10124: Faster simplejob start

jtamate added a comment. False alarm. :-) Do not know what caused it, but after a complete kdesrc-build, the leak is gone. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks, mwolff Cc: mwolff, broulik, ngraham, anthonyfieroni, michaelh

D10124: Faster simplejob start

jtamate added a comment. In https://phabricator.kde.org/D10124#199645, @mwolff wrote: > can you test and make sure that q and slave actually get destroyed? If that is the case, then the slotobj should get destroyed too. If not then this is a bug in Qt, which would make me wonder... Do yo

D10124: Faster simplejob start

mwolff added a comment. can you test and make sure that q and slave actually get destroyed? If that is the case, then the slotobj should get destroyed too. If not then this is a bug in Qt, which would make me wonder... Do you have a selfcompiled Qt, or one shipped by your distro? There was h

D10124: Faster simplejob start

jtamate added a comment. In https://phabricator.kde.org/D10124#199601, @mwolff wrote: > only that one, not the others? strange, why should that be the case? are you sure `q` and `slave` get destroyed? Yes, only that one. I've tried with const parámeters in the lambdas, same probl

D10124: Faster simplejob start

mwolff added a comment. only that one, not the others? strange, why should that be the case? are you sure `q` and `slave` get destroyed? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks, mwolff Cc: mwolff, broulik, ngraham, anthonyfieroni,

D10124: Faster simplejob start

jtamate added a comment. Unfortunately, the last connect creates a memory leak: 1018== 10,152 (72 direct, 10,080 indirect) bytes in 1 blocks are definitely lost in loss record 928 of 943 ---

D10124: Faster simplejob start

jtamate marked 7 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks, mwolff Cc: mwolff, broulik, ngraham, anthonyfieroni, michaelh

D10124: Faster simplejob start

jtamate updated this revision to Diff 26239. jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a comment. Thanks to mwolf, broulik & anthonyfieroni. It works now. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.k

D10124: Faster simplejob start

mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. the crash may be due to missing context, the three-arg connect shouldn't ever be used imo INLINE COMMENTS > simplejob.cpp:141 > > -q->connect(slave, SIGNAL(connected()), >

D10124: Faster simplejob start

jtamate updated this revision to Diff 26109. jtamate added a comment. Reverting to first patch. The lambda syntax has some problems with private d_func pointers under dolphin Thread 1 "dolphin" received signal SIGSEGV, Segmentation fault. 0x721b5b4c in KJob::d_func (this=0x4

D10124: Faster simplejob start

jtamate updated this revision to Diff 26047. jtamate added a comment. - Faster simplejob start Finally the C++11 lambda syntax worked. Now it only uses 0.3% of cpu according to callgrind. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10124?vs=26011&id=2

D10124: Faster simplejob start

broulik added inline comments. INLINE COMMENTS > simplejob.cpp:142 > > q->connect(slave, SIGNAL(infoMessage(QString)), > SLOT(_k_slotSlaveInfoMessage(QString))); Try something along the lines of q->connect(slave, &Slave::infoMessage, q, std::bind(&SimpleJobPrivate::_k_

D10124: Faster simplejob start

anthonyfieroni added a comment. This -> https://wiki.qt.io/New_Signal_Slot_Syntax#Overload REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks Cc: ngraham, anthonyfieroni

D10124: Faster simplejob start

jtamate added a comment. In https://phabricator.kde.org/D10124#196534, @anthonyfieroni wrote: > Is there any problems to change other connects too? There is only one problem, that I do not know how to do it. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10

D10124: Faster simplejob start

jtamate created this revision. jtamate added a reviewer: Frameworks. Restricted Application added a project: Frameworks. jtamate requested review of this revision. REVISION SUMMARY Changing only 4 of the signals connections to the new syntax almost doubles the spped of simplejob::start in a ki

D10124: Faster simplejob start

anthonyfieroni added a comment. Is there any problems to change other connects too? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks Cc: anthonyfieroni

D10124: Faster simplejob start

jtamate added a comment. F5675857: kcioclient5_move_2000_files_old_connect.png F5675856: kcioclient5_move_2000_files_partly_connect.png From spending 11.65% to 7.06% REPOSITORY R241 KIO REVISION DETAIL