<table><tr><td style="">meven accepted this revision.<br />meven added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D28745">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D28745#649011" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D28745#649011</a>, <a href="https://phabricator.kde.org/p/marcingu/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@marcingu</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D28745#648036" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D28745#648036</a>, <a href="https://phabricator.kde.org/p/meven/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@meven</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>This is gonna have an hefty toll on perf as it will add a <tt style="background: #ebebeb; font-size: 13px;">getmntent</tt> syscall to every thumbnail generation.<br />
Using <tt style="background: #ebebeb; font-size: 13px;">Solid::Device::listFromType</tt> would leverage Solid always up-to-date (using events rather thane sysalls) device cache.<br />
I am not sure in the end this is preferable though.</p></div>
</blockquote>
<p>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.</p></div>
</blockquote>
<p>AFAICT this is not clear at the moment. This is too bad it stumbles upon a new contributor to deal with it.<br />
I recently learned about QStorageInfo that covers the same use case as KMountPoint.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>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.<br />
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.</p></blockquote>
<p>We should avoid absolutely avoid that, this is far worse than saving 1 syscall per thumbnail.</p>
<p>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.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28745#inline-164707">View Inline</a><span style="color: #4b4d51; font-weight: bold;">thumbnail.cpp:729</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">auto</span> <span class="n">thumbRootMount</span> <span style="color: #aa2211">=</span> <span class="n">mountsList</span><span class="p">.</span><span class="n">findByPath</span><span class="p">(</span><span class="n">m_thumbBasePath</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">//If file is on the same filesystem as thumbnail directory, we can cache it.</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">bool</span> <span class="n">isEncrypted</span> <span style="color: #aa2211">=</span> <span class="n">mount</span> <span style="color: #aa2211">!=</span> <span class="n">thumbRootMount</span> <span style="color: #aa2211">&&</span> <span class="p">(</span><span class="n">mount</span><span style="color: #aa2211">-></span><span class="n">mountType</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"fuse.cryfs"</span><span class="p">)</span> <span style="color: #aa2211">||</span> <span class="n">mount</span><span style="color: #aa2211">-></span><span class="n">mountType</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"fuse.encfs"</span><span class="p">));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Add a space after //</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R320 KIO Extras</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28745">https://phabricator.kde.org/D28745</a></div></div><br /><div><strong>To: </strong>marcingu, ivan, broulik, Dolphin, ngraham, meven<br /><strong>Cc: </strong>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<br /></div>