DevHeads.net

Review Request: Cache icon and accessibility in KFilePlacesItem

Review request for kdelibs.

Description
Similar to what Dolphin does in the "Places" pane, the open/save dialog should cache devices' icons, emblems and whether it's accessible, rather then querying Solid::Device all the time. Querying can cause Solid to ask udisksd to enumerate all devices (because with udisksd2 you can't ask for just a single device), which can take a lot of time (especially when slow devices like NFS shares or slow USB sticks are present).

Since this is a model item, the values are queried quite a lot, causing high CPU load from udiskds and the calling application blocks. Just try to quickly move mouse over Solid devices in the open/save dialog and what your CPU load).

Most visible result of this caching is that the dialog is presented faster and you can smoothly resize the "Places" pane.

Lukas Tinkl will try to optimize the Solid Udisks backend a bit to make it even faster, as this problem with device icons causing querying udisksd has impact on Dolphin start up time (and with this patch it has similar impact on the dialog show-up time).

Diffs
kfile/kfileplacesitem.cpp 3ada0c3
kfile/kfileplacesitem_p.h 3c6bd30

Diff: <a href="http://git.reviewboard.kde.org/r/107030/diff/" title="http://git.reviewboard.kde.org/r/107030/diff/">http://git.reviewboard.kde.org/r/107030/diff/</a>

Testing

Thanks,

Dan Vrátil

Comments

Re: Review Request: Cache icon and accessibility in KFilePlacesI

By Commit Hook at 10/25/2012 - 04:48

This review has been submitted with commit f29be6d309a533e0870b0d693adc5c2468938b6e by Dan Vrátil to branch KDE/4.9.

- Commit Hook

On Oct. 24, 2012, 9:13 p.m., Dan Vrátil wrote:

Re: Review Request: Cache icon and accessibility in KFilePlacesI

By Christoph Feck at 10/24/2012 - 18:33

Ship it!

On a second look, I see no cluttering.

- Christoph Feck

On Oct. 24, 2012, 9:13 p.m., Dan Vrátil wrote:

Re: Review Request: Cache icon and accessibility in KFilePlacesI

By Christoph Feck at 10/24/2012 - 18:26

Let me understand, what you are proposing. If Solid is slow returning the values, then the caching should be in Solid, not in every application that uses it. But until that is implemented in Solid, we could use this workaround in kfile.

If that is right, I wonder what time frame is expected for a possible Solid fix. I am not familar with internals in Solid, but a patch there should be of similar complexity as this one, and I would rather wait for it instead of cluttering kfile with a workaround. If the Solid fix, however, is not expected before 4.10, then "Ship it!"

- Christoph Feck

On Oct. 24, 2012, 9:13 p.m., Dan Vrátil wrote:

Re: Review Request: Cache icon and accessibility in KFilePlacesI

By Dan Vratil at 10/25/2012 - 04:52

If the fix is possible, it will be most probably in 4.9.x. However calling repeatedly Solid still causes some overhead. This change is quite small, has positive impact on performance and does not really make the code more complex, so I pushed it.

We can always drop it from 4.10.

- Dan

On Oct. 24, 2012, 9:13 p.m., Dan Vrátil wrote: