Adding new benchmark to test image decoding performance.

bench/nanobench.cpp

Index: bench/nanobench.cpp
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index eb48ec228d6b35886d6c8a87cd5e1c344f9479e3..2e126e29026bb471c7f965ed6ccded384946cbc2 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -9,10 +9,13 @@
 
 #include "Benchmark.h"
 #include "CrashHandler.h"
+#include "DecodingBench.h"
+#include "DecodingSubsetBench.h"
 #include "GMBench.h"
 #include "ProcStats.h"
 #include "ResultsWriter.h"
 #include "RecordingBench.h"
+#include "Resources.h"
 #include "SKPBench.h"
 #include "Stats.h"
 #include "Timer.h"
@@ -439,7 +442,11 @@ public:
                       , fCurrentRecording(0)
                       , fCurrentScale(0)
                       , fCurrentSKP(0)
-                      , fCurrentUseMPD(0) {
+                      , fCurrentUseMPD(0)
+                      , fCurrentImage(0)
+                      , fCurrentSubsetImage(0)
+                      , fCurrentColorType(0)
+                      , fDivisor(2) {
         for (int i = 0; i < FLAGS_skps.count(); i++) {
             if (SkStrEndsWith(FLAGS_skps[i], ".skp")) {
                 fSKPs.push_back() = FLAGS_skps[i];
@@ -469,6 +476,27 @@ public:
         if (FLAGS_mpd) {
             fUseMPDs.push_back() = true;
         }
+        
+        // Prepare the images for decoding
+        for (int i = 0; i < FLAGS_images.count(); i++) {
+            const char* flag = FLAGS_images[i];
+            if (sk_isdir(flag)) {
+                // If the value passed in is a directory, add all the images
+                SkOSFile::Iter it(flag);
+                SkString file;
+                while (it.next(&file)) {
+                    fImages.push_back() = SkOSPath::Join(flag, file.c_str());
+                }
+            } else if (sk_exists(flag)) {
+                // Also add the value if it is a single image
+                fImages.push_back() = flag;
+            }
+        }
+        
+        // Choose the candidate color types for image decoding
+        const SkColorType colorTypes[] =
+            { kN32_SkColorType, kRGB_565_SkColorType, kAlpha_8_SkColorType };
+        fColorTypes.push_back_n(SK_ARRAY_COUNT(colorTypes), colorTypes);
     }
 
     static bool ReadPicture(const char* path, SkAutoTUnref<SkPicture>* pic) {
@@ -562,6 +590,76 @@ public:
             fCurrentScale++;
         }
 
+        // Run the DecodingBenches
+        SkString file;
+        while (fCurrentImage < fImages.count()) {
+            while (fCurrentColorType < fColorTypes.count()) {
+                const SkString& path = fImages[fCurrentImage];
+                SkColorType colorType = fColorTypes[fCurrentColorType];
+                fCurrentColorType++;
+                // Check if the image decodes before creating the benchmark
+                SkBitmap bitmap;
+                if (SkImageDecoder::DecodeFile(path.c_str(), &bitmap,
+                        colorType, SkImageDecoder::kDecodePixels_Mode)) {
+                    return SkNEW_ARGS(DecodingBench, (path, colorType));
+                }
+            }
+            fCurrentColorType = 0;
+            fCurrentImage++;
+        }
+
+        // Run the DecodingSubsetBenches
+        while (fCurrentSubsetImage < fImages.count()) {
+            while (fCurrentColorType < fColorTypes.count()) {
+                const SkString& path = fImages[fCurrentSubsetImage];
+                SkColorType colorType = fColorTypes[fCurrentColorType];
+                fCurrentColorType++;
+                // Check if the image decodes before creating the benchmark
+                SkAutoTUnref<SkData> encoded(
+                        SkData::NewFromFileName(path.c_str()));
+                SkAutoTDelete<SkMemoryStream> stream(
+                        new SkMemoryStream(encoded));
+                SkAutoTDelete<SkImageDecoder>
+                    decoder(SkImageDecoder::Factory(stream.get()));
+                if (!decoder) {
+                    SkDebugf("Cannot find decoder for %s\n", path.c_str());

[scroggo, 2015/02/12 19:53:28]

Why do we make sure decoding succeeds for subset images, but not for regular
decoding?

[msarett, 2015/02/12 22:05:45]

We do in fact make the check for both.  The check is much shorter for decode
because it is encapsulated by DecodeFile.

+                } else {
+                    stream->rewind();
+                    int w, h;
+                    bool success;
+                    if (!decoder->buildTileIndex(stream.detach(), &w, &h)
+                            || w*h == 1) {
+                        SkDebugf("Subset decoding is not supported for %s\n",

[msarett, 2015/02/12 18:57:06]

Forgot to mention: We may want to consider failing silently here.  This error
message is printed many times if the images in the specified directory aren't
webp.

[mtklein, 2015/02/12 19:28:56]

Ah.  We can certainly fail less loudly.  Maybe only print this the first time? 
The nanobench driver is singlethreaded, so this can be a global bool or
something... expedient hacks work fine.

[msarett, 2015/02/12 19:42:51]

Done.

+                                path.c_str());
+                        success = false;
+                    } else if (fDivisor > w || fDivisor > h) {
+                        SkDebugf("Divisor %d is too big for %s %dx%d\n",
+                                fDivisor, path.c_str(), w, h);
+                        success = false;
+                    } else {
+                        const int sW  = w / fDivisor;
+                        const int sH = h / fDivisor;
+                        SkBitmap bitmap;
+                        success = true;
+                        for (int y = 0; y < h; y += sH) {

[scroggo, 2015/02/12 19:53:28]

At first it seemed odd to me that we make sure all subsets succeed, but if one
of the subset decodes failed we could get a misleading answer, so this probably
makes sense.

We now have this code in 3 places. I think it would be better to consolidate it
somewhere. (This also means we'll be able to update it more easily.)

[msarett, 2015/02/12 22:05:45]

It  is true that we have similar code in 3 places which is unfortunate.

The code in nanobench.cpp creates a stream, builds the tile index, and decodes
subsets.  It prints any error and moves to the next benchmark.

The code in DMSrcSick.cpp performs the same tasks, however it returns any error
as a string.

The code in DecodingSubsetBench.cpp performs these tasks as well, however it has
some key differences from the first two.  There is no error checking because we
assume any errors affecting the benchmark would have been caught in nanobench
(and we don't want to include error checking in the benchmark timing). 
Additionally, it splits up the tasks between the constructor and onDraw, since
we only want to measure the performance of the decode.

I don't think it makes sense to share code between DecodingSubsetBench.cpp and
the first two.  It seems to me that there are too many differences.

I think that we could share between nanobench.cpp and DMSrcSink.cpp by adding an
additional API to SkImageDecoder.cpp (called DecodeMemorySubset or something)
that returns some kind of error code that could be printed in the case of
nanobench and returned in the case of DMSrcSink.cpp.

However, I'm not sure how much sense this makes given that we are in the process
of changing APIs anyway.  I think this would be worthwhile to discuss.

+                            for (int x = 0; x < w; x += sW) {
+                                SkIRect rect = SkIRect::MakeXYWH(x, y, sW, sH);
+                                success &= decoder->decodeSubset(&bitmap, rect,
+                                                                 colorType);
+                            }
+                        }
+                    }
+                    // Create the benchmark if successful
+                    if (success) {
+                        return SkNEW_ARGS(DecodingSubsetBench,

[scroggo, 2015/02/12 19:53:28]

Any reason you're going back and forth on "new" versus "SkNEW"? Typically inside
the Skia library we use SkNEW. I think our tests are inconsistent, but I thought
we had decided we didn't need it there?

[msarett, 2015/02/12 22:05:45]

I am happy to not use SkNew!

+                                (path, colorType, fDivisor));
+                    }
+                }
+            }
+            fCurrentColorType = 0;
+            fCurrentSubsetImage++;
+        }
+
         return NULL;
     }
 
@@ -591,6 +689,8 @@ private:
     SkTArray<SkScalar> fScales;
     SkTArray<SkString> fSKPs;
     SkTArray<bool>     fUseMPDs;
+    SkTArray<SkString> fImages;
+    SkTArray<SkColorType> fColorTypes;
 
     double fSKPBytes, fSKPOps;
 
@@ -600,6 +700,10 @@ private:
     int fCurrentScale;
     int fCurrentSKP;
     int fCurrentUseMPD;
+    int fCurrentImage;
+    int fCurrentSubsetImage;
+    int fCurrentColorType;
+    const int fDivisor;
 };
 
 int nanobench_main();