Merge branch 'master' of ssh://git.debian.org/git/collab-maint/shotwell

1 files changed, 171 insertions, 79 deletions

diff --git a/src/Photo.vala b/src/Photo.vala
index 34b2676..98a3175 100644
--- a/src/Photo.vala
+++ b/src/Photo.vala
@@ -199,7 +199,11 @@ public abstract class Photo : PhotoSource, Dateable {
 
     // The number of seconds we should hold onto a precached copy of the original image; if
     // it hasn't been accessed in this many seconds, discard it to conserve memory.
-    private const int PRECACHE_TIME_TO_LIVE = 180;
+    private const int SOURCE_PIXBUF_TIME_TO_LIVE_SEC = 10;
+    
+    // min and max size of source pixbuf cache LRU
+    private const int SOURCE_PIXBUF_MIN_LRU_COUNT = 1;
+    private const int SOURCE_PIXBUF_MAX_LRU_COUNT = 3;
     
     // Minimum raw embedded preview size we're willing to accept; any smaller than this, and 
     // it's probably intended primarily for use only as a thumbnail and won't look good on the
@@ -293,6 +297,35 @@ public abstract class Photo : PhotoSource, Dateable {
         public PhotoFileReader editable;
     }
     
+    private class CachedPixbuf {
+        public Photo photo;
+        public Gdk.Pixbuf pixbuf;
+        public Timer last_touched = new Timer();
+        
+        public CachedPixbuf(Photo photo, Gdk.Pixbuf pixbuf) {
+            this.photo = photo;
+            this.pixbuf = pixbuf;
+        }
+    }
+    
+    // The first time we have to run the pipeline on an image, we'll precache
+    // a copy of the unscaled, unmodified version; this allows us to operate
+    // directly on the image data quickly without re-fetching it at the top
+    // of the pipeline, which can cause significant lag with larger images.
+    //
+    // This adds a small amount of (automatically garbage-collected) memory
+    // overhead, but greatly simplifies the pipeline, since scaling can now
+    // be blithely ignored, and most of the pixel operations are fast enough
+    // that the app remains responsive, even with 10MP images.
+    //
+    // In order to make sure we discard unneeded precaches in a timely fashion,
+    // we spawn a timer when the unmodified pixbuf is first precached; if the
+    // timer elapses and the pixbuf hasn't been needed again since then, we'll
+    // discard it and free up the memory.  The cache also has an LRU to prevent
+    // runaway amounts of memory from being stored (see SOURCE_PIXBUF_LRU_COUNT)
+    private static Gee.LinkedList<CachedPixbuf>? source_pixbuf_cache = null;
+    private static uint discard_source_id = 0;
+    
     // because fetching individual items from the database is high-overhead, store all of
     // the photo row in memory
     protected PhotoRow row;
@@ -308,23 +341,6 @@ public abstract class Photo : PhotoSource, Dateable {
     private OneShotScheduler remove_editable_scheduler = null;
     
     protected bool can_rotate_now = true;
-
-    // The first time we have to run the pipeline on an image, we'll precache
-    // a copy of the unscaled, unmodified version; this allows us to operate
-    // directly on the image data quickly without re-fetching it at the top
-    // of the pipeline, which can cause significant lag with larger images.
-    //
-    // This adds a small amount of (automatically garbage-collected) memory
-    // overhead, but greatly simplifies the pipeline, since scaling can now
-    // be blithely ignored, and most of the pixel operations are fast enough
-    // that the app remains responsive, even with 10MP images.
-    //
-    // In order to make sure we discard unneeded precaches in a timely fashion,
-    // we spawn a timer when the unmodified pixbuf is first precached; if the
-    // timer elapses and the pixbuf hasn't been needed again since then, we'll
-    // discard it and free up the memory.
-    private Gdk.Pixbuf unmodified_precached = null;
-    private GLib.Timer secs_since_access = null;
     
     // RAW only: developed backing photos.
     private Gee.HashMap<RawDeveloper, BackingPhotoRow?>? developments = null;
@@ -455,6 +471,19 @@ public abstract class Photo : PhotoSource, Dateable {
         cached_exposure_time = this.row.exposure_time;
     }
     
+    protected static void init_photo() {
+        source_pixbuf_cache = new Gee.LinkedList<CachedPixbuf>();
+    }
+    
+    protected static void terminate_photo() {
+        source_pixbuf_cache = null;
+        
+        if (discard_source_id != 0) {
+            Source.remove(discard_source_id);
+            discard_source_id = 0;
+        }
+    }
+    
     protected virtual void notify_editable_replaced(File? old_file, File? new_file) {
         editable_replaced(old_file, new_file);
     }
@@ -785,7 +814,7 @@ public abstract class Photo : PhotoSource, Dateable {
             if (!is_raw_developer_complete(d)) {
                 develop_photo(d);
                 try {
-                    populate_prefetched();
+                    get_prefetched_copy();
                 } catch (Error e) {
                     // couldn't reload the freshly-developed image, nothing to display
                     return;
@@ -826,7 +855,7 @@ public abstract class Photo : PhotoSource, Dateable {
         }
         
         notify_altered(new Alteration("image", "developer"));
-        discard_prefetched(true);
+        discard_prefetched();
     }
     
     public RawDeveloper get_raw_developer() {
@@ -3204,64 +3233,123 @@ public abstract class Photo : PhotoSource, Dateable {
     public override Gdk.Pixbuf get_pixbuf(Scaling scaling) throws Error {
         return get_pixbuf_with_options(scaling);
     }
-
+    
     /**
-     * @brief Populates the cached version of the unmodified image.
+     * One-stop shopping for the source pixbuf cache.
+     *
+     * The source pixbuf cache holds untransformed, unscaled (full-sized) pixbufs of Photo objects.
+     * These can be rather large and shouldn't be held in memory for too long, nor should many be
+     * allowed to stack up.
+     *
+     * If locate is non-null, a source pixbuf is returned for the Photo.  If keep is true, the
+     * pixbuf is stored in the cache.  (Thus, passing a Photo w/ keep == false will drop the cached
+     * pixbuf.)  If Photo is non-null but keep is false, null is returned.
+     *
+     * Whether locate is null or not, the cache is walked in its entirety, dropping expired pixbufs
+     * and dropping excessive pixbufs from the LRU.  Locating a Photo "touches" the pixbuf, i.e.
+     * it moves to the head of the LRU.
      */
-    public void populate_prefetched() throws Error {
-        lock (unmodified_precached) {
-            // If we don't have it already, precache the original...
-            if (unmodified_precached == null) {
-                unmodified_precached = load_raw_pixbuf(Scaling.for_original(), Exception.ALL, BackingFetchMode.SOURCE);
-                secs_since_access = new GLib.Timer();
-                GLib.Timeout.add_seconds(5, (GLib.SourceFunc)discard_prefetched);
-                debug("spawning new precache timeout for %s", this.to_string()); 
+    private static Gdk.Pixbuf? run_source_pixbuf_cache(Photo? locate, bool keep) throws Error {
+        lock (source_pixbuf_cache) {
+            CachedPixbuf? found = null;
+            
+            // walk list looking for photo to locate (if specified), dropping expired and LRU'd
+            // pixbufs along the way
+            double min_elapsed = double.MAX;
+            int count = 0;
+            Gee.Iterator<CachedPixbuf> iter = source_pixbuf_cache.iterator();
+            while (iter.next()) {
+                CachedPixbuf cached_pixbuf = iter.get();
+                
+                double elapsed = Math.trunc(cached_pixbuf.last_touched.elapsed()) + 1;
+                
+                if (locate != null && cached_pixbuf.photo.equals(locate)) {
+                    // found it, remove and reinsert at head of LRU (below)...
+                    iter.remove();
+                    found = cached_pixbuf;
+                    
+                    // ...that's why the counter is incremented
+                    count++;
+                } else if (elapsed >= SOURCE_PIXBUF_TIME_TO_LIVE_SEC) {
+                    iter.remove();
+                } else if (count >= SOURCE_PIXBUF_MAX_LRU_COUNT) {
+                    iter.remove();
+                } else {
+                    // find the item with the least elapsed time to reschedule a cache trim (to
+                    // prevent onesy-twosy reschedules)
+                    min_elapsed = double.min(elapsed, min_elapsed);
+                    count++;
+                }
+            }
+            
+            // if not found and trying to locate one and keep it, generate now
+            if (found == null && locate != null && keep) {
+                found = new CachedPixbuf(locate,
+                    locate.load_raw_pixbuf(Scaling.for_original(), Exception.ALL, BackingFetchMode.SOURCE));
+            } else if (found != null) {
+                // since it was located, touch it so it doesn't expire
+                found.last_touched.start();
+            }
+            
+            // if keeping it, insert at head of LRU
+            if (found != null && keep) {
+                source_pixbuf_cache.insert(0, found);
+                
+                // since this is (re-)inserted, count its elapsed time too ... w/ min_elapsed, this
+                // is almost guaranteed to be the min, since it was was touched mere clock cycles
+                // ago...
+                min_elapsed = double.min(found.last_touched.elapsed(), min_elapsed);
+                
+                // ...which means don't need to readjust the min_elapsed when trimming off excess
+                // due to adding back an element
+                while(source_pixbuf_cache.size > SOURCE_PIXBUF_MAX_LRU_COUNT)
+                    source_pixbuf_cache.poll_tail();
             }
+            
+            // drop expiration timer...
+            if (discard_source_id != 0) {
+                Source.remove(discard_source_id);
+                discard_source_id = 0;
+            }
+            
+            // ...only reschedule if there's something to expire
+            if (source_pixbuf_cache.size > SOURCE_PIXBUF_MIN_LRU_COUNT) {
+                assert(min_elapsed >= 0.0);
+                
+                // round-up to avoid a bunch of zero-second timeouts
+                uint retry_sec = SOURCE_PIXBUF_TIME_TO_LIVE_SEC - ((uint) Math.trunc(min_elapsed));
+                discard_source_id = Timeout.add_seconds(retry_sec, trim_source_pixbuf_cache, Priority.LOW);
+            }
+            
+            return found != null ? found.pixbuf : null;
         }
     }
-
+    
+    private static bool trim_source_pixbuf_cache() {
+        try {
+            run_source_pixbuf_cache(null, false);
+        } catch (Error err) {
+        }
+        
+        return false;
+    }
+    
     /**
      * @brief Get a copy of what's in the cache.
      *
-     * @return A Pixbuf with the image data from unmodified_precached.
+     * @return A copy of the Pixbuf with the image data from unmodified_precached.
      */
     public Gdk.Pixbuf get_prefetched_copy() throws Error {
-        lock (unmodified_precached) {
-            if (unmodified_precached == null) {
-                try {
-                    populate_prefetched();
-                } catch (Error e) {
-                    message("pixbuf for %s could not be loaded: %s", to_string(), e.message);
-                    
-                    throw e;
-                }
-            }
-
-            return unmodified_precached.copy();
-        }
+        return run_source_pixbuf_cache(this, true).copy();
     }
 
     /**
      * @brief Discards the cached version of the unmodified image.
-     *
-     * @param immed Whether the cached version should be discarded now, or not.
      */
-    public bool discard_prefetched(bool immed = false) {
-        lock (unmodified_precached) {
-            if (secs_since_access == null)
-                return false;
-            
-            double tmp;
-            if ((secs_since_access.elapsed(out tmp) > PRECACHE_TIME_TO_LIVE) || (immed)) {
-                debug("pipeline not run in over %d seconds or got immediate command, discarding " + 
-                    "cached original for %s",
-                    PRECACHE_TIME_TO_LIVE, to_string());
-                unmodified_precached = null;
-                secs_since_access = null;
-                return false;
-            }
-
-            return true;
+    public void discard_prefetched() {
+        try {
+            run_source_pixbuf_cache(this, false);
+        } catch (Error err) {
         }
     }
     
@@ -3283,7 +3371,7 @@ public abstract class Photo : PhotoSource, Dateable {
         Timer timer = new Timer();
         Timer total_timer = new Timer();
         double redeye_time = 0.0, crop_time = 0.0, adjustment_time = 0.0, orientation_time = 0.0,
-            straighten_time = 0.0;
+            straighten_time = 0.0, scale_time = 0.0;
 
         total_timer.start();
 #endif
@@ -3329,13 +3417,8 @@ public abstract class Photo : PhotoSource, Dateable {
         //
         // Image load-and-decode
         //
-        populate_prefetched();
-
-        Gdk.Pixbuf pixbuf = get_prefetched_copy();
         
-        // remember to delete the cached copy if it isn't being used.
-        secs_since_access.start();
-        debug("pipeline being run against %s, timer restarted.", this.to_string());
+        Gdk.Pixbuf pixbuf = get_prefetched_copy();
         
         //
         // Image transformation pipeline
@@ -3405,15 +3488,15 @@ public abstract class Photo : PhotoSource, Dateable {
 #endif
         }
         
-#if MEASURE_PIPELINE
-        debug("PIPELINE %s (%s): redeye=%lf crop=%lf adjustment=%lf orientation=%lf total=%lf",
-            to_string(), scaling.to_string(), redeye_time, crop_time, adjustment_time, 
-            orientation_time, total_timer.elapsed());
-#endif
-
         // scale the scratch image, as needed.
         if (is_scaled) {
+#if MEASURE_PIPELINE
+            timer.start();
+#endif
             pixbuf = pixbuf.scale_simple(scaled_to_viewport.width, scaled_to_viewport.height, Gdk.InterpType.BILINEAR);
+#if MEASURE_PIPELINE
+            scale_time = timer.elapsed();
+#endif
         }
 
         // color adjustment; we do this dead last, since, if an image has been scaled down,
@@ -3434,7 +3517,13 @@ public abstract class Photo : PhotoSource, Dateable {
         // the pixbuf, and must be accounted for the test to be valid.
         if ((is_scaled) && (!is_straightened))
             assert(scaled_to_viewport.approx_equals(Dimensions.for_pixbuf(pixbuf), SCALING_FUDGE));
-
+        
+#if MEASURE_PIPELINE
+        debug("PIPELINE %s (%s): redeye=%lf crop=%lf adjustment=%lf orientation=%lf straighten=%lf scale=%lf total=%lf",
+            to_string(), scaling.to_string(), redeye_time, crop_time, adjustment_time,
+            orientation_time, straighten_time, scale_time, total_timer.elapsed());
+#endif
+        
         return pixbuf;
     }
 
@@ -4032,12 +4121,12 @@ public abstract class Photo : PhotoSource, Dateable {
 
         // at this point, any image date we have cached is stale,
         // so delete it and force the pipeline to re-fetch it
-        discard_prefetched(true);
+        discard_prefetched();
     }
     
     private void on_reimport_editable() {
         // delete old image data and force the pipeline to load new from file.
-        discard_prefetched(true);
+        discard_prefetched();
         
         debug("Reimporting editable for %s", to_string());
         try {
@@ -4890,6 +4979,8 @@ public class LibraryPhoto : Photo, Flaggable, Monitorable {
     }
     
     public static void init(ProgressMonitor? monitor = null) {
+        init_photo();
+        
         global = new LibraryPhotoSourceCollection();
         
         // prefetch all the photos from the database and add them to the global collection ...
@@ -4921,6 +5012,7 @@ public class LibraryPhoto : Photo, Flaggable, Monitorable {
     }
     
     public static void terminate() {
+        terminate_photo();
     }
     
     // This accepts a PhotoRow that was prepared with Photo.prepare_for_import and