---- Reported by lucas@yorba.org 2013-04-12 15:32:00 -0700 ----

Original Redmine bug id: 6812
Original URL: http://redmine.yorba.org/issues/6812
Searchable id: yorba-bug-6812
Original author: Lucas Beeler
Original description:

From Joseph Bylund on the mailing list:

> Hi all,

>

> I did a little experimentation with profiling raw imports (you can find an
example sysprof profile here: http://ubuntuone.com/6PrDbY32fdP2QXT2g8LSbf).
And found that libraw is actually the limiting factor during imports while
using the camera developer (probably 75%+ of time). Is it reasonable to file a
bug against slow importing?

>

> thanks,

-Joe

The amount of CPU time libraw is using seems ridiculously high when importing
with the RAW developer set to Camera mode. In Camera mode, the libraw pipeline
shouldn't be run at all. Instead, we should merely pluck the embedded JPEG out
of the RAW file. This is just reading bytes and shouldn't be CPU-intensive at
all. However, I suspect that even in Camera mode, libraw still runs the full
RAW development pipeline on each image. This is a serious performance problem.
We should investigate and fix it.

Related issues:
related to shotwell - 6745: With the developer set to 'Camera', when
importing an RAW... (Open)
duplicated by shotwell - Feature #7416: Importing 1100 RAW files takes forever
with CPU hot, low IO (Duplicate)



---- Additional Comments From shotwell-maint@gnome.bugs 2013-08-29 11:55:00 -0700 ----

### History

####

#1

Updated by Joe Bylund 6 months ago

Looks like the raw is being developed for the thumbnails at
ThumbnailCache.generate_for_photo from Photo.prepare_for_import.

####

#2

Updated by Jim Nelson 6 months ago

  * **Target version** deleted (<strike>_0.15.0_</strike>)

####

#3

Updated by Joe Bylund 3 months ago

  * **File** generate_for_photo_alt.vala added

What do you think of in ThumbnailCache.generate_for_photo only getting one
thumbnail, at the largest size, and then downscaling that thumbnail to make
the others? This eliminates one of raw load/unpack/read's. I'm attaching a
copy of the function so you know what I'm talking about. While this makes this
function a lot faster for me (2x) I'm still not seeing any improvement in the
overall import speed.

####

#4

Updated by Jim Nelson 3 months ago

  * **Target version** set to _0.15.0_

I think this is certainly a direction worth considering. If you looked back
over the course of time at this method, you will see it bouncing back and
forth between two approaches: loading the image scaled twice, or loading the
image scaled at one size then scaling it down for the second. With normal JPEG
this was a bit of a wash, but the second scaling meant the thumbnails were
slightly less crisp, so we went with two scaled load-and-decodes. But with
RAW, the slight improvement may not be worth it.

I'm unsurprised it didn't help the overall import time -- Shotwell import has
been highly parallelized and speeding up one task doesn't necessarily improve
the overall time. The problem with this approach is that it taxes the system
more than some users would like.

In any event, this seems to be an improvement. Could you turn this into a
patch and attach here?

####

#5

Updated by Joe Bylund 3 months ago

  * **File** 6812_remove_double_read.patch added

What about doing the scaled read at twice the size of the largest thumbnail,
this should mean that all the thumbnails are pretty crisp, though maybe not as
good as starting from full size every time. It still eliminates one
load/unpack/scaled_read for raw files, and doesn't resort to an unscaled read
(which is slower for raw's due to a different, more expensive,
demoasicing/interpolation).

I'd think leave it open, but bump it to 0.16.

Also, is it kosher to treat a size as an int like this?

-Joe

####

#6

Updated by Jim Nelson 3 months ago

  * **Status** changed from _Open_ to _Review_

I would do something like this:

    
    
    int size = (int) Size.BIG * 2;
    

Those are enums for a reason, even if their values are set to their pixel
sizes.

An easy change for me to make when I review this. No need for another patch,
unless you'd like to clean it up yourself.

####

#7

Updated by Jim Nelson 3 months ago

I see now why you can't make that an int so easily; the Size enum's scaling
function is called.

I'd rather not muck around too much, so I added a comment and left it as-is.

Thanks!

####

#8

Updated by Anonymous 3 months ago

  * **Status** changed from _Review_ to _Fixed_

Applied in changeset 3a424909653f77e1e337ac3e1aad803b49f04942.

####

#9

Updated by Jim Nelson 3 months ago

  * **Status** changed from _Fixed_ to _Open_
  * **Target version** changed from _0.15.0_ to _0.16.0_

Joe has asked that I re-open, as he feels the performance can be improved even
further.



--- Bug imported by chaz@yorba.org 2013-11-25 22:11 UTC  ---

This bug was previously known as _bug_ 6812 at http://redmine.yorba.org/show_bug.cgi?id=6812
Imported an attachment (id=262790)
Imported an attachment (id=262791)

Unknown milestone "unknown in product shotwell. 
   Setting to default milestone for this product, "---".
Setting qa contact to the default for this product.
   This bug either had no qa contact or an invalid one.
Resolution set on an open status.
   Dropping resolution 
CC member joseph.bylund+shotwell@gmail.com does not have an account here
The original submitter of attachment 262790 [details] was
   joseph.bylund+shotwell@gmail.com, but he doesn't have an account here.
   Reassigning to the person who moved it here: chaz@yorba.org.
The original submitter of attachment 262791 [details] [review] was
   joseph.bylund+shotwell@gmail.com, but he doesn't have an account here.
   Reassigning to the person who moved it here: chaz@yorba.org.