Sorting is making a lot of QCollator objects, making it slow

[Mark Gaiser]

On Sat, Feb 3, 2018 at 1:41 PM, Mark Gaiser <markg85 at gmail.com> wrote:

> Hi,
>
> I was doing some benchmarks for drag and drop for [1] and noticed that the
> QCollator constructor was called _a_lot_ of times. The test was for 5000
> files in a split view (so that would be 10.000 files) with QCollator being
> called well over 81.000 times.
>
> I do remember that we once had only one QCollator object prior to sorting
> being parallel. Then parallel sorting came around and a trick was done by
> introducing the KFileItemModelLessThan class which would manage the
> QCollator lifetime for thread safety. This is all as far as i remember it
> happening, i could be slightly off but it should be along those lines.
>
> Now in benchmarks i see a lot of QCollator(QLocale) copy constructor
> calls. That can't be right, can it?
>
> Now i partly reverted the work that was done back then (just as an
> experiment) to see if this stuff can be done more efficiently. I removed
> the QCollator completely from KFileItemModelLessThan with the rationale
> of KFileItemModelLessThan ultimatately just calling lessThan on the
> provided model which is then being executed through QCollator::compare. The
> Qt docs call QCollator reentrant (not thread-safe), but QCollator::compare
> is a const function (aka, promising the compiler to not change class
> internals) which in my mind makes that particular function thread-safe,
> right?
>
> You can find my patch here [2].
>
> Now the fun stuff. It works 50% of the time...
> The other 50% it crashes in NonMovableFileItem. Ouch.
> I don't know why that happens, i'm hoping some of you can take a look with
> this patch [2] applied? Or is my reasoning about QCollator::compare wrong
> and it that causing the crash?
>
> Note: the 50% that it does work does feel a lot faster (the sorting that
> is). So this does matter.
>
>
> Best regards,
> Mark
>
> [1] https://phabricator.kde.org/D10085
> [2] https://p.sc2.nl/BkKVKXmIG
>

5 days without a response, i guess this went under the radar of the folks
it was intended for.
Howdy, David and Frank :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180208/3e0ad0fe/attachment.htm>