<div dir="ltr"><div>I have to say that I'm really enjoying the commentary. Thanks for the taking the time to not only fix stuff, but also to write it up!<br><br></div>Perhaps I need to get out more. <br></div><div class="gmail_extra"><br><div class="gmail_quote">On 18 May 2018 at 13:20, Robert Krawitz <span dir="ltr"><<a href="mailto:rlk@alum.mit.edu" target="_blank">rlk@alum.mit.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">There are some really serious problems in the thumbnail cache code:<br>
<br>
1) It misuses timers. Timers can only be started and stopped in the<br>
same thread. Even putting them all in the same function is not<br>
good enough, if that function can be called from different threads;<br>
with the multi-threaded nature of thumbnail building, calling<br>
ThumbnailCache::save() directly means it can be called from<br>
multiple threads.<br>
<br>
The practical impact is that the 1-second timer for thumbnail<br>
saving accomplished nothing, because the timer was seldom getting<br>
started (or stopped).<br>
<br>
The solution is to never call save() directly; always call it<br>
indirectly through emitting a signal.<br>
<br>
2) The thumbnail cache itself is saved very inefficiently; it's<br>
written in full to a temporary file, which is then copied (yes,<br>
copied, not renamed) to the real file. Even with a big database,<br>
such as my own (270,303 images) that's probably not killing me with<br>
I/O, but it's a lot of work to be doing every 100 images.<br>
<br>
I'm changing it to do an incremental save where possible. That<br>
technically violates the intent of the file format, which stores<br>
the most recently written file and offset as well as the image<br>
count, but those can be reconstituted on cache reload easily<br>
enough.<br>
<br>
Also, in order to do that, I had to add an additional list of<br>
unsaved files (auto-save list). And there's no good reason for the<br>
data structure to be a QMap; a QHash is faster, and ordering<br>
doesn't matter in the cache.<br>
<br>
3) The save() routine wasn't protected by a mutex; with multiple<br>
threads processing thumbnails, I'm not sure how we didn't get a lot<br>
of data corruption in the index cache.<br>
<br>
I'm working on all of these problems.<br>
<br>
-- <br>
Robert Krawitz <<a href="mailto:rlk@alum.mit.edu">rlk@alum.mit.edu</a>><br>
<br>
*** MIT Engineers A Proud Tradition <a href="http://mitathletics.com" rel="noreferrer" target="_blank">http://mitathletics.com</a> ***<br>
Member of the League for Programming Freedom -- <a href="http://ProgFree.org" rel="noreferrer" target="_blank">http://ProgFree.org</a><br>
Project lead for Gutenprint -- <a href="http://gimp-print.sourceforge.net" rel="noreferrer" target="_blank">http://gimp-print.sourceforge.<wbr>net</a><br>
<br>
"Linux doesn't dictate how I work, I dictate how Linux works."<br>
--Eric Crampton<br>
______________________________<wbr>_________________<br>
KPhotoAlbum mailing list<br>
<a href="mailto:KPhotoAlbum@mail.kdab.com">KPhotoAlbum@mail.kdab.com</a><br>
<a href="https://mail.kdab.com/mailman/listinfo/kphotoalbum" rel="noreferrer" target="_blank">https://mail.kdab.com/mailman/<wbr>listinfo/kphotoalbum</a><br>
</blockquote></div><br></div>