ImageDecodeBench packeted runs should resume decoding where it left off

third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp

 // Copyright 2015 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.
 
 // Provides a minimal wrapping of the Blink image decoders. Used to perform
 // a non-threaded, memory-to-memory image decode using micro second accuracy
 // clocks to measure image decode time. Optionally applies color correction
 // during image decoding on supported platforms (default off). Usage:
 //
 //   % ninja -C out/Release image_decode_bench &&
@@ skipping 208 matching lines @@
     for (int i = 0; i < frame_count; ++i) {
       if (!decoder->FrameBufferAtIndex(i))
         return false;
     }
 
     return !decoder->Failed();
   }
 
   RefPtr<SharedBuffer> packet_data = SharedBuffer::Create();
   size_t position = 0;
+  size_t next_frame_to_decode = 0;
   while (true) {
     const char* packet;
     size_t length = data->GetSomeData(packet, position);
 
     length = std::min(length, packet_size);
     packet_data->Append(packet, length);
     position += length;
 
     bool all_data_received = position == data->size();
     decoder->SetData(packet_data.Get(), all_data_received);
 
     int frame_count = decoder->FrameCount();
-    for (int i = 0; i < frame_count; ++i) {
-      if (!decoder->FrameBufferAtIndex(i))
+    for (int i = next_frame_to_decode; i < frame_count; ++i) {

[scroggo, 2017/05/11 13:56:23]

FWIW, starting from frame 0 doesn't re-decode those first three frames (one
exception - when purging aggressively, they *will* need to be re-decoded). We
don't clear the cache, so they should still be there and complete, so
FrameBufferAtIndex will just return the complete frames.

But you're right, no need to call FrameBufferAtIndex for those frames
(especially for the exception of purging aggressively).

I don't think this change goes far enough, though. FrameCount() should only
return frames that we at least have a piece of, and FrameBufferAtIndex only
returns null if i >= FrameCount(). So I don't think we'll ever hit this case
(meaning that next_frame_to_decode will remain 0). Instead, I think you want
something like the following:

  for (; next_frame_to_decode < frame_count; ++next_frame_to_decode) {
    auto status =
decoder->FrameBufferAtIndex(next_frame_to_decode)->GetStatus();
    if (status != ImageFrame::FrameComplete) {
      break;
    }

[cblume, 2017/05/11 15:45:31]

Ah. I agree.

Since I believe this test is aimed at replicating real-world behavior, I might
want to add the purging as well. I don't want to time it (once I fix the
timing). But the act of purging could impact instruction and data caches. What
do you think?

Also Leon, do you prefer I add your google address, chromium address, or both? I
added your chromium address because you've given LGTMs from it before. But maybe
you want both?

+      if (!decoder->FrameBufferAtIndex(i)) {
+        next_frame_to_decode = i;
         break;
+      }
     }
 
     if (all_data_received || decoder->Failed())
       break;
   }
 
   return !decoder->Failed();
 }
 
 }  // namespace
@@ skipping 93 matching lines @@
   double average_time = total_time / static_cast<double>(iterations);
   printf("%f %f\n", total_time, average_time);
   return 0;
 }
 
 }  // namespace blink
 
 int main(int argc, char* argv[]) {
   return blink::Main(argc, argv);
 }