Review Request: Load Solid devices in KFilePlacesModel asynchronously

[Mark Gaiser]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107513/#review22795
-----------------------------------------------------------


Ehm, this just seems odd to me. Because kdelibs is frozen you add a completely new class and public even. Thus you already change the kdelibs api which should not be allowed.

Second argument, if this udisks2 is so important to have, why don't we just make an exception to the frozen api and allow some changes in. I'm sure the kde folks couldn't have known that udisks2 was going to be widely used within the "kdelibs is frozen, bug fixes only" timeline. This is just out of the kdelibs hands. Again, an exception would better.

Third argument, adding this as public API and wanting other applications to use this till frameworks is done will just add a lot more work on the app side. First to change the current stuff and add the cache class then later on to revert to how the code looks now because that class is temporary and won't be in frameworks. Really, seriously? 

Why is udisks2 slower anyway?

- Mark Gaiser


On Nov. 29, 2012, 2:55 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107513/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 2:55 p.m.)
> 
> 
> Review request for kdelibs, Lukáš Tinkl and David Faure.
> 
> 
> Description
> -------
> 
> With KDE 4.10 Solid will provide udisks2 backend. Unfortunately udisks2 is somewhat slower then udisks1 and listing all the devices when starting Gwenview or file dialog takes more time then is acceptable for users. I only have a single optical drive and one HDD with 4 partitions and it takes ~2 seconds for the file dialog to appear and udisks2d causes high CPU spikes during that. And it gets much worse with slow, cheap USB sticks etc.
> 
> This patch adds an asynchronous cache, which will call Solid::Devices::listFromQuery() in a thread and then notify listeners through deviceAdded() signal about all the devices received. The cache keeps itself up-to-date (via Solid::DeviceNotifier) and notifies listeners about these changes via deviceAdded() and deviceRemoved() signals. This patch also ports the KFilePlacesModel to use the cache which results in KFileDialog appearing immediately. Gwenview also starts faster.
> 
> The KFilePlacesDeviceCache class is exported as public because of Dolphin, which has it's own implementation of places model similar to KFilePlacesModel and which suffers from the same problems (and possibly there are other apps). We have already verified that porting Dolphin's model to this cache resolves the issue. Better solution would be for libsolid to have asynchronous API, however that's not possible now with kdelibs being feature-frozen. With libsolid2, this class can go away. Meanwhile however it's the only way to fix this performance regression that will hit every distro that will decide to ditch udisks1 with KDE 4.10 and switch to udisks2.
> 
> 
> Diffs
> -----
> 
>   kfile/CMakeLists.txt ceae140 
>   kfile/kfileplacesdevicecache.h PRE-CREATION 
>   kfile/kfileplacesdevicecache.cpp PRE-CREATION 
>   kfile/kfileplacesmodel.cpp 0192926 
> 
> Diff: http://git.reviewboard.kde.org/r/107513/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121129/8a2eb5a5/attachment.htm>