[KPhotoAlbum] More "loading" efficiency issues

[Robert Krawitz]

I found two more loading (in)efficiency issues:

1) When converting RAW files into images, the loader tries to use the
   embedded preview if available.  This in turn uses
   KDcraw::loadRawPreview(), which internally converts it from JPEG to
   QImage.  Assuming that it uses libjpeg, it does so inefficiently
   because it makes a lot of system calls, reading the image in small
   chunks, and as a result doing a lot of extra work.  I changed it
   (on the load performance branch) to use the much more efficient
   in-memory JPEG decoding.

   This has no direct effect on wall-clock performance on my system
   that I'm aware of unless I'm reading from the SSD using 2 or more
   scouts.  However, it greatly reduces the user and system CPU time,
   so people with lots of RAW files and either slower CPUs or very
   fast storage will benefit, and in any event it will hit the system
   less hard (both in CPU time and IOPS) when loading RAW files.

   The result of this is that MD5 computation actually takes over 50%
   of the user CPU time (estimated by kcachegrind) when loading RAW
   files, and building thumbnails takes about 44% of the time on my
   system.  But given that I'm seeing CPU consumption under 20%
   (vs. about 50%) while more or less pinning the I/O rate, it should
   be able to take pretty good advantage of NVMe or other fast
   storage.

2) I wanted to see if there was still anything using the slow path
   JPEG loader.  When I put debug messages in, and went into the
   viewer and quickly ran through that, I was surprised to see a lot
   of files being queued and loaded repeatedly.  It looks like the
   problem is that ImageManager::AsyncLoader::loadImage() is not
   correctly detecting whether a request is already queued.  The
   problem is that the list of images being loaded is a
   QSet<ImageRequest*> rather than QSet<ImageRequest&> or the like, so
   it does a pointer comparison rather than a value comparison.  The
   patch below does a value comparison, unfortunately having to
   iterate over the queue to find a duplicate request.

   There are enough uses of ImageRequest that I didn't want to
   completely rewrite the logic.

   Unfortunately, with or without this change, the viewer can get
   behind the loader, in which case the viewer freezes for a few
   seconds until it reaches an active request in the queue.  What we
   really need to do is something like put high priority requests on
   the front of the queue or something, but that's going to be a
   harder problem.

   I'm going to commit this separately from #1, so if it turns out to
   cause problems, we can pull it out easily.

diff --git a/ImageManager/AsyncLoader.cpp b/ImageManager/AsyncLoader.cpp
index e7f81223..84698add 100644
--- a/ImageManager/AsyncLoader.cpp
+++ b/ImageManager/AsyncLoader.cpp
@@ -134,6 +134,17 @@ void ImageManager::AsyncLoader::loadImage( ImageRequest* request )
         return; // We are currently loading it, calm down and wait please ;-)
     }
 
+    // Try harder to find a pending request.  Unfortunately, we can't simply use
+    // m_currentLoading.contains() because that will compare pointers
+    // when we want to compare values.
+    for (req = m_currentLoading.begin(); req != m_currentLoading.end(); req++) {
+        ImageRequest *r = *req;
+        if (*request == *r) {
+            delete request;
+            return; // We are currently loading it, calm down and wait please ;-)
+        }
+    }
+            
     // if request is "fresh" (not yet pending):
     if (m_loadList.addRequest( request ))
         m_sleepers.wakeOne();


-- 
Robert Krawitz                                     <rlk at alum.mit.edu>

***  MIT Engineers   A Proud Tradition   http://mitathletics.com  ***
Member of the League for Programming Freedom  --  http://ProgFree.org
Project lead for Gutenprint   --    http://gimp-print.sourceforge.net

"Linux doesn't dictate how I work, I dictate how Linux works."
--Eric Crampton