Split Convolver.SIMDVerification into two tests

skia/ext/convolver_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string.h>
 #include <time.h>
 #include <algorithm>
 #include <numeric>
 #include <vector>
 
@@ skipping 194 matching lines @@
   filter.AddFilter(66, factors6, arraysize(factors6));
   ASSERT_EQ(6, filter.max_filter());
   ASSERT_EQ(6, filter.num_values());
 
   values = filter.FilterForValue(5, &filter_offset, &filter_length);
   ASSERT_TRUE(values == NULL);   // filter_length == 0 => values is NULL
   ASSERT_EQ(66, filter_offset);  // value passed in
   ASSERT_EQ(0, filter_length);
 }
 
-#if defined(THREAD_SANITIZER)
-// Times out under ThreadSanitizer, http://crbug.com/134400.
-#define MAYBE_SIMDVerification DISABLED_SIMDVerification
-#else
-#define MAYBE_SIMDVerification SIMDVerification
-#endif
-TEST(Convolver, MAYBE_SIMDVerification) {
-  int source_sizes[][2] = {
-    {1,1}, {1,2}, {1,3}, {1,4}, {1,5},
-    {2,1}, {2,2}, {2,3}, {2,4}, {2,5},
-    {3,1}, {3,2}, {3,3}, {3,4}, {3,5},
-    {4,1}, {4,2}, {4,3}, {4,4}, {4,5},
-#ifdef NDEBUG
-    {1920, 1080},
-    {720, 480},
-    {1377, 523},
-#endif
-    {325, 241}
-};
-#ifdef NDEBUG
-  int dest_sizes[][2] = { {1280, 1024}, {480, 270}, {177, 123} };
-#else
-  int dest_sizes[][2] = { {128, 102}, {48, 27}, {17, 13} };
-#endif
+void VerifySIMD(unsigned int source_width,
+                unsigned int source_height,
+                unsigned int dest_width,
+                unsigned int dest_height) {
   float filter[] = { 0.05f, -0.15f, 0.6f, 0.6f, -0.15f, 0.05f };
+  // Preparing convolve coefficients.
+  ConvolutionFilter1D x_filter, y_filter;
+  for (unsigned int p = 0; p < dest_width; ++p) {
+    unsigned int offset = source_width * p / dest_width;
+    EXPECT_LT(offset, source_width);
+    x_filter.AddFilter(offset, filter,
+                       std::min<int>(arraysize(filter),
+                                     source_width - offset));
+  }
+  x_filter.PaddingForSIMD();
+  for (unsigned int p = 0; p < dest_height; ++p) {
+    unsigned int offset = source_height * p / dest_height;
+    y_filter.AddFilter(offset, filter,
+                       std::min<int>(arraysize(filter),
+                                     source_height - offset));
+  }
+  y_filter.PaddingForSIMD();
+
+  // Allocate input and output skia bitmap.
+  SkBitmap source, result_c, result_sse;
+  source.setConfig(SkBitmap::kARGB_8888_Config,
+                   source_width, source_height);
+  source.allocPixels();
+  result_c.setConfig(SkBitmap::kARGB_8888_Config,
+                     dest_width, dest_height);
+  result_c.allocPixels();
+  result_sse.setConfig(SkBitmap::kARGB_8888_Config,
+                       dest_width, dest_height);
+  result_sse.allocPixels();
+
+  // Randomize source bitmap for testing.
+  unsigned char* src_ptr = static_cast<unsigned char*>(source.getPixels());
+  for (int y = 0; y < source.height(); y++) {
+    for (unsigned int x = 0; x < source.rowBytes(); x++)
+      src_ptr[x] = rand() % 255;
+    src_ptr += source.rowBytes();
+  }
+
+  // Test both cases with different has_alpha.
+  for (int alpha = 0; alpha < 2; alpha++) {
+    // Convolve using C code.
+    base::TimeTicks resize_start;
+    base::TimeDelta delta_c, delta_sse;
+    unsigned char* r1 = static_cast<unsigned char*>(result_c.getPixels());
+    unsigned char* r2 = static_cast<unsigned char*>(result_sse.getPixels());
+
+    resize_start = base::TimeTicks::Now();
+    BGRAConvolve2D(static_cast<const uint8*>(source.getPixels()),
+                   static_cast<int>(source.rowBytes()),
+                   (alpha != 0), x_filter, y_filter,
+                   static_cast<int>(result_c.rowBytes()), r1, false);
+    delta_c = base::TimeTicks::Now() - resize_start;
+
+    resize_start = base::TimeTicks::Now();
+    // Convolve using SSE2 code
+    BGRAConvolve2D(static_cast<const uint8*>(source.getPixels()),
+                   static_cast<int>(source.rowBytes()),
+                   (alpha != 0), x_filter, y_filter,
+                   static_cast<int>(result_sse.rowBytes()), r2, true);
+    delta_sse = base::TimeTicks::Now() - resize_start;
+
+    // Unfortunately I could not enable the performance check now.
+    // Most bots use debug version, and there are great difference between
+    // the code generation for intrinsic, etc. In release version speed
+    // difference was 150%-200% depend on alpha channel presence;
+    // while in debug version speed difference was 96%-120%.
+    // TODO(jiesun): optimize further until we could enable this for
+    // debug version too.
+    // EXPECT_LE(delta_sse, delta_c);
+
+    int64 c_us = delta_c.InMicroseconds();
+    int64 sse_us = delta_sse.InMicroseconds();
+    VLOG(1) << "from:" << source_width << "x" << source_height
+            << " to:" << dest_width << "x" << dest_height
+            << (alpha ? " with alpha" : " w/o alpha");
+    VLOG(1) << "c:" << c_us << " sse:" << sse_us;
+    VLOG(1) << "ratio:" << static_cast<float>(c_us) / sse_us;
+
+    // Comparing result.
+    for (unsigned int i = 0; i < dest_height; i++) {
+      EXPECT_FALSE(memcmp(r1, r2, dest_width * 4)); // RGBA always
+      r1 += result_c.rowBytes();
+      r2 += result_sse.rowBytes();
+    }
+  }
+}
+
+TEST(Convolver, VerifySIMDEdgeCases) {
+  srand(static_cast<unsigned int>(time(0)));
+  // Loop over all possible (small) image sizes
+  for (unsigned int width = 1; width < 20; width++) {
+    for (unsigned int height = 1; height < 20; height++) {
+      VerifySIMD(width, height, 8, 8);
+      VerifySIMD(8, 8, width, height);
+    }
+  }
+}
+
+// Verify that lage upscales/downscales produce the same result
+// with and without SIMD.
+TEST(Convolver, VerifySIMDPrecision) {
+  int source_sizes[][2] = { {1920, 1080}, {1377, 523}, {325, 241} };
+  int dest_sizes[][2] = { {1280, 1024}, {177, 123} };

[Stephen White, 2014/02/25 21:13:59]

I'm surprised these run in reasonable time in DEBUG. The big image combinations
were taking approximately 45 seconds before, and intermittently falling over the
45 second timeout, leading to flakiness. But perhaps that was only due to the
small (edge cases) being resized to the large destination sizes, or because
you've removed the 640x480 destination size. 

At any rate, please keep an eye on the runtimes on the bots when this lands,
since they may have weaker hardware than what you're running locally.

[hubbe, 2014/02/25 21:59:18]

A good chunk of time was spent comparing the images I think.
With ASAN, on my machine, replacing the comparison with memcmp saved 50% of the
time...

 
   srand(static_cast<unsigned int>(time(0)));
 
   // Loop over some specific source and destination dimensions.
   for (unsigned int i = 0; i < arraysize(source_sizes); ++i) {
     unsigned int source_width = source_sizes[i][0];
     unsigned int source_height = source_sizes[i][1];
     for (unsigned int j = 0; j < arraysize(dest_sizes); ++j) {
       unsigned int dest_width = dest_sizes[j][0];
       unsigned int dest_height = dest_sizes[j][1];
-
-      // Preparing convolve coefficients.
-      ConvolutionFilter1D x_filter, y_filter;
-      for (unsigned int p = 0; p < dest_width; ++p) {
-        unsigned int offset = source_width * p / dest_width;
-        EXPECT_LT(offset, source_width);
-        x_filter.AddFilter(offset, filter,
-                           std::min<int>(arraysize(filter),
-                                         source_width - offset));
-      }
-      x_filter.PaddingForSIMD();
-      for (unsigned int p = 0; p < dest_height; ++p) {
-        unsigned int offset = source_height * p / dest_height;
-        y_filter.AddFilter(offset, filter,
-                           std::min<int>(arraysize(filter),
-                                         source_height - offset));
-      }
-      y_filter.PaddingForSIMD();
-
-      // Allocate input and output skia bitmap.
-      SkBitmap source, result_c, result_sse;
-      source.setConfig(SkBitmap::kARGB_8888_Config,
-                       source_width, source_height);
-      source.allocPixels();
-      result_c.setConfig(SkBitmap::kARGB_8888_Config,
-                         dest_width, dest_height);
-      result_c.allocPixels();
-      result_sse.setConfig(SkBitmap::kARGB_8888_Config,
-                           dest_width, dest_height);
-      result_sse.allocPixels();
-
-      // Randomize source bitmap for testing.
-      unsigned char* src_ptr = static_cast<unsigned char*>(source.getPixels());
-      for (int y = 0; y < source.height(); y++) {
-        for (unsigned int x = 0; x < source.rowBytes(); x++)
-          src_ptr[x] = rand() % 255;
-        src_ptr += source.rowBytes();
-      }
-
-      // Test both cases with different has_alpha.
-      for (int alpha = 0; alpha < 2; alpha++) {
-        // Convolve using C code.
-        base::TimeTicks resize_start;
-        base::TimeDelta delta_c, delta_sse;
-        unsigned char* r1 = static_cast<unsigned char*>(result_c.getPixels());
-        unsigned char* r2 = static_cast<unsigned char*>(result_sse.getPixels());
-
-        resize_start = base::TimeTicks::Now();
-        BGRAConvolve2D(static_cast<const uint8*>(source.getPixels()),
-                       static_cast<int>(source.rowBytes()),
-                       (alpha != 0), x_filter, y_filter,
-                       static_cast<int>(result_c.rowBytes()), r1, false);
-        delta_c = base::TimeTicks::Now() - resize_start;
-
-        resize_start = base::TimeTicks::Now();
-        // Convolve using SSE2 code
-        BGRAConvolve2D(static_cast<const uint8*>(source.getPixels()),
-                       static_cast<int>(source.rowBytes()),
-                       (alpha != 0), x_filter, y_filter,
-                       static_cast<int>(result_sse.rowBytes()), r2, true);
-        delta_sse = base::TimeTicks::Now() - resize_start;
-
-        // Unfortunately I could not enable the performance check now.
-        // Most bots use debug version, and there are great difference between
-        // the code generation for intrinsic, etc. In release version speed
-        // difference was 150%-200% depend on alpha channel presence;
-        // while in debug version speed difference was 96%-120%.
-        // TODO(jiesun): optimize further until we could enable this for
-        // debug version too.
-        // EXPECT_LE(delta_sse, delta_c);
-
-        int64 c_us = delta_c.InMicroseconds();
-        int64 sse_us = delta_sse.InMicroseconds();
-        VLOG(1) << "from:" << source_width << "x" << source_height
-                << " to:" << dest_width << "x" << dest_height
-                << (alpha ? " with alpha" : " w/o alpha");
-        VLOG(1) << "c:" << c_us << " sse:" << sse_us;
-        VLOG(1) << "ratio:" << static_cast<float>(c_us) / sse_us;
-
-        // Comparing result.
-        for (unsigned int i = 0; i < dest_height; i++) {
-          for (unsigned int x = 0; x < dest_width * 4; x++) {  // RGBA always.
-            EXPECT_EQ(r1[x], r2[x]);
-          }
-          r1 += result_c.rowBytes();
-          r2 += result_sse.rowBytes();
-        }
-      }
+      VerifySIMD(source_width, source_height, dest_width, dest_height);
     }
   }
 }
 
 TEST(Convolver, SeparableSingleConvolution) {
   static const int kImgWidth = 1024;
   static const int kImgHeight = 1024;
   static const int kChannelCount = 3;
   static const int kStrideSlack = 22;
   ConvolutionFilter1D filter;
@@ skipping 185 matching lines @@
                               fp_gradient_kernel.end()), -1.5f);
   EXPECT_LT(*std::min_element(fp_gradient_kernel.begin(),
                               fp_gradient_kernel.end()), 0.0f);
   EXPECT_LT(*std::max_element(fp_gradient_kernel.begin(),
                               fp_gradient_kernel.end()), 1.5f);
   EXPECT_GT(*std::max_element(fp_gradient_kernel.begin(),
                               fp_gradient_kernel.end()), 0.0f);
 }
 
 }  // namespace skia