Changing the signals connections to the new syntax. From being expensive to not being visible in kcachegrind.
Details
- Reviewers
mwolff dfaure - Group Reviewers
Frameworks - Commits
- R241:42fed2e70a21: Faster simplejob start
kioclient5 move of 3000 files and move of 3000 files in dolphin to different disks (no rename).
Diff Detail
- Repository
- R241 KIO
- Branch
- simplejob (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage
src/core/simplejob.cpp | ||
---|---|---|
138–139 | Try something along the lines of q->connect(slave, &Slave::infoMessage, q, std::bind(&SimpleJobPrivate::_k_slotSlaveInfoMessage, this); Note that if there's a disconnect for that particular signal somewhere in the code this will not work but at a quick glance I can only see a disconnect(m_slave); which is fine QObject::connect is static, so you might as well do QObject::connect instead of q->connect |
- Faster simplejob start
Finally the C++11 lambda syntax worked.
Now it only uses 0.3% of cpu according to callgrind.
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.
0x00007ffff21b5b4c in KJob::d_func (this=0x4000000000000000) at /g/5kde/frameworks/kcoreaddons/src/lib/jobs/kjob.h:651
651 Q_DECLARE_PRIVATE(KJob)
(gdb) where
#0 0x00007ffff21b5b4c in KJob::d_func (this=0x4000000000000000) at /g/5kde/frameworks/kcoreaddons/src/lib/jobs/kjob.h:651
#1 0x00007ffff21b3a77 in KJob::totalAmount (this=0x4000000000000000, unit=KJob::Bytes) at /g/5kde/frameworks/kcoreaddons/src/lib/jobs/kjob.cpp:235
#2 0x00007ffff39a22f3 in KIO::SimpleJobPrivate::slotTotalSize (this=0xd4de80, size=2097152) at /g/5kde/frameworks/kio/src/core/simplejob.cpp:268
#3 0x00007ffff39a1332 in KIO::SimpleJobPrivate::<lambda(KIO::filesize_t)>::operator()(KIO::filesize_t) const (__closure=0xdd0bc0, size=2097152)
at /g/5kde/frameworks/kio/src/core/simplejob.cpp:153
#4 0x00007ffff39a4120 in QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<long long unsigned int>, void, KIO::SimpleJobPrivate::start(KIO::Slave*)::<lambda(KIO::filesize_t)> >::call(KIO::SimpleJobPrivate::<lambda(KIO::filesize_t)> &, void **) (f=..., arg=0x7fffffffcc50)
at /usr/include/qt5/QtCore/qobjectdefs_impl.h:130
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
1018== at 0x4C2E6FF: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
1018== by 0xCAEFCFC: QObjectPrivate::connectImpl(QObject const*, int, QObject const*, void**, QtPrivate::QSlotObjectBase*, Qt::ConnectionType, int const*, QMetaObject const*) (in /usr/lib64/libQt5Core.so.5.10.0)
1018== by 0xCAF0084: QObject::connectImpl(QObject const*, void, QObject const*, void, QtPrivate::QSlotObjectBase*, Qt::ConnectionType, int const*, QMetaObject const*) (in /usr/lib64/libQt5Core.so.5.10.0)
1018== by 0x9141BB5: std::enable_if<QtPrivate::FunctionPointer<KIO::SimpleJobPrivate::start(KIO::Slave*)::{lambda(unsigned long long)#4}>::ArgumentCount==(-1), QMetaObject::Connection>::type QObject::connect<void (KIO::SlaveInterface::*)(unsigned long long), KIO::SimpleJobPrivate::start(KIO::Slave*)::{lambda(unsigned long long)#4}>(QtPrivate<void (KIO::SlaveInterface::*)(unsigned long long)>::Object const*, std::enable_if<QtPrivate::FunctionPointer<KIO::SimpleJobPrivate::start(KIO::Slave*)::{lambda(unsigned long long)#4}>::ArgumentCount==(-1), QMetaObject::Connection>::type, QObject const*, QtPrivate::FunctionPointer, Qt::ConnectionType) (qobject.h:338)
1018== by 0x913FCCB: KIO::SimpleJobPrivate::start(KIO::Slave*) (simplejob.cpp:151)
1018== by 0x914B3D1: KIO::TransferJobPrivate::start(KIO::Slave*) (transferjob.cpp:362)
1018== by 0x914D063: startJob(KIO::SimpleJob*, KIO::Slave*) (scheduler.cpp:60)
1018== by 0x914F661: KIO::ProtoQueue::startAJob() (scheduler.cpp:635)
1018== by 0x9152AC4: KIO::ProtoQueue::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_scheduler_p.cpp:252)
1018== by 0xCAEBDB9: QMetaObject::activate(QObject*, int, int, void**) (in /usr/lib64/libQt5Core.so.5.10.0)
1018== by 0xCAF8236: QTimer::timeout(QTimer::QPrivateSignal) (in /usr/lib64/libQt5Core.so.5.10.0)
1018== by 0xCAF8567: QTimer::timerEvent(QTimerEvent*) (in /usr/lib64/libQt5Core.so.5.10.0)
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 problem.
And https://forum.qt.io/topic/86962/qt5-new-signal-to-lambda-connections-can-result-memory-leak/3 says the leaks were fixed in Qt5.0.1
How to reproduce:
apply the patch :-)
valgrind --log-file=whatever.log dolphin (that uses the compiled kio)
copy&paste one file and drag&drop one file.
Exit and check whatever.log
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 https://codereview.qt-project.org/#/c/215333/ but that's for QMetaObject::invoke which isn't relevant here... Odd!
Here doesn't happen!. Only in the other Linux. I'll try again next week in the other linux.
Both linux have Qt from OpenSuse.
False alarm. :-)
Do not know what caused it, but after a complete kdesrc-build, the leak is gone.
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.)
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 ;)