meven accepted this revision. meven added a comment.
In D28745#649011 <https://phabricator.kde.org/D28745#649011>, @marcingu wrote: > In D28745#648036 <https://phabricator.kde.org/D28745#648036>, @meven wrote: > > > This is gonna have an hefty toll on perf as it will add a `getmntent` syscall to every thumbnail generation. > > Using `Solid::Device::listFromType` would leverage Solid always up-to-date (using events rather thane sysalls) device cache. > > I am not sure in the end this is preferable though. > > > Unfortunately I'm new to the project and have no idea what would be the best way of checking if path is on encrypted filesystem. AFAICT this is not clear at the moment. This is too bad it stumbles upon a new contributor to deal with it. I recently learned about QStorageInfo that covers the same use case as KMountPoint. > One thing I could do is to move check into thumbForDirectory and transfer it through drawSubThumbnail and createSubThumbnail. Which will run the check once per directory, instead of running it up to four times. > The big downside however is that we no longer will be able to skip the check if thumbnail already exist, so that likely would be slower in most cases. We should avoid absolutely avoid that, this is far worse than saving 1 syscall per thumbnail. And after a second though a sysall is not much given we are generating a thumbnail that will be far longer to compute, so the price of the syscall should be unnoticeable. INLINE COMMENTS > thumbnail.cpp:729 > + const auto thumbRootMount = > mountsList.findByPath(m_thumbBasePath); > + //If file is on the same filesystem as thumbnail directory, we > can cache it. > + const bool isEncrypted = mount != thumbRootMount && > (mount->mountType() == QLatin1String("fuse.cryfs") || mount->mountType() == > QLatin1String("fuse.encfs")); Add a space after // REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D28745 To: marcingu, ivan, broulik, #dolphin, ngraham, meven Cc: meven, ngraham, kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, rdieter, mikesomov
