D10857: Change qSort to std::sort in simplifiedUrlList

2018-03-02 Thread Mark Gaiser
markg added a comment. In D10857#216725 , @dfaure wrote: > I cannot confirm that stable_sort is faster, on the contrary. http://www.davidfaure.fr/2018/qsort_performance.cpp updated, says (repeatedly) > "std::sort took: 5003 ms" >

D10857: Change qSort to std::sort in simplifiedUrlList

2018-03-02 Thread Jaime Torres Amate
jtamate added a comment. I've found the problem. The problem is qSort itself. Chaning qSort to qStableSort I've got normal times in the test case, select 50.000 files and pressing Shift+Supr. Also in the small test, the times, running under callgrind are: qSort: between 33148 and

D10857: Change qSort to std::sort in simplifiedUrlList

2018-02-27 Thread Mark Gaiser
markg added a comment. In D10857#214867 , @jtamate wrote: > In D10857#214607 , @dfaure wrote: > > > I'm not opposed to the idea, but measuring CPU usage is a rather misleading indicator. What if it

D10857: Change qSort to std::sort in simplifiedUrlList

2018-02-27 Thread Aleix Pol Gonzalez
apol added a comment. I'd say the bottom line is qSort is deprecated in favor of std::sort. @jtamate make sure you are profiling release builds. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10857 To: jtamate, #frameworks, dfaure Cc: markg, apol, michaelh

D10857: Change qSort to std::sort in simplifiedUrlList

2018-02-26 Thread Jaime Torres Amate
jtamate added a comment. In D10857#214607 , @dfaure wrote: > I'm not opposed to the idea, but measuring CPU usage is a rather misleading indicator. What if it takes 10 times longer, because it's progressing much more slowly? ;) > > Please

D10857: Change qSort to std::sort in simplifiedUrlList

2018-02-26 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I'm not opposed to the idea, but measuring CPU usage is a rather misleading indicator. What if it takes 10 times longer, because it's progressing much more slowly? ;) Please

D10857: Change qSort to std::sort in simplifiedUrlList

2018-02-26 Thread Mark Gaiser
markg added a comment. +1, also for what @apol said. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10857 To: jtamate, #frameworks, dfaure Cc: markg, apol, michaelh

D10857: Change qSort to std::sort in simplifiedUrlList

2018-02-26 Thread Aleix Pol Gonzalez
apol added a comment. +1 If there's 11, we better change them all at once? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10857 To: jtamate, #frameworks, dfaure Cc: apol, michaelh

D10857: Change qSort to std::sort in simplifiedUrlList

2018-02-26 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: Frameworks, dfaure. Restricted Application added a project: Frameworks. jtamate requested review of this revision. REVISION SUMMARY qSort is depecreated in Qt5. But qSort is also quite slow compared to std::sort. There are 11 more