[lipstick] Process icon additions and removals in batches to improve performance #100
Conversation
| bool itemsRemoved = false; | ||
|
|
||
| if (!items.isEmpty()) { | ||
| beginResetModel(); |
There was a problem hiding this comment.
Hmm. Unless the QML is so bad that re-creating all delegates is cheaper than messing around with them (!), it should be much better to emit added/removed signals by row.
The QML views combine changes and apply them all at once in the next iteration of the event loop. It should be fairly efficient. If there's a layout per item changed or anything like that, something is very wrong.
There was a problem hiding this comment.
There is something like that, because the UI freezes for 30 seconds when removing 400 items. I could of course come up with a separate test case for this. Adding seems much faster.
There was a problem hiding this comment.
For any regular use of views, this should not happen. I'd be interested in seeing a testcase, I'd be willing to bet it's the use of height: contentHeight causing delegates to be created for everything (which we really need to move away from, anyhow).
From that perspective (though by all means let's look at a testcase) I'd like to avoid this part of the change, because while it may make our (bad) case worse, it pessimises well-written view use.
There was a problem hiding this comment.
...similarly, if you order 'items' by row index, and move backwards (from the highest row index), you could try to figure out the optimal signals to emit and turn this into a minimal number of beginRemoveRows/endRemoveRows.. it's tricky, but worth it if you can make it happen. that should then be quite efficient.
I have written code for this before, if you ask me, I can help you with it tomorrow
|
suggestion: split this into two changes. the batching of processing (looks reasonably good to go), and the model black magic (with possible reworking) |
|
Interestingly, my standalone test case gives totally different results. I'll investigate this a bit more... |
Currently adding a .desktop file in /usr/share/applications causes that file to be processed and added to the model immediately. Likewise, removing a file causes it to be removed from the model immediately. Unfortunately the QML GridView, for example, isn't very efficient at handling additions and especially not removals. Removing multiple items at the same time one by one takes a lot of time: removing 400 .desktop files at once will cause the UI to freeze for over 30 seconds on a moderately quick system. To work around this, this patch does additions and removals in batches if no files have changed during the last half a second. When doing so, it also resets the model completely if more than one item has changed, instead of changing the model one by one (causing the GridView, for example, to relayout all the items).
For 400 items this shaves off about a second or two from the addition time and about 27 seconds from the removal time.