Improve stability of the echo detector complexity perf tests.

webrtc/modules/audio_processing/residual_echo_detector_complexity_unittest.cc

Index: webrtc/modules/audio_processing/residual_echo_detector_complexity_unittest.cc
diff --git a/webrtc/modules/audio_processing/residual_echo_detector_complexity_unittest.cc b/webrtc/modules/audio_processing/residual_echo_detector_complexity_unittest.cc
index f1cf6fa3f8dd09e615d80a05ab747c197a135022..b36e2dfc0a4d342929184bab691a14a7989f1547 100644
--- a/webrtc/modules/audio_processing/residual_echo_detector_complexity_unittest.cc
+++ b/webrtc/modules/audio_processing/residual_echo_detector_complexity_unittest.cc
@@ -26,9 +26,12 @@
 namespace webrtc {
 namespace {
 
-const size_t kNumFramesToProcess = 500;
-const size_t kProcessingBatchSize = 20;
-const size_t kWarmupBatchSize = 2 * kProcessingBatchSize;
+const size_t kNumFramesToProcess = 20000;

[hlundin-webrtc, 2017/03/17 08:48:03]

Nit: constexprs, please.

[ivoc, 2017/03/17 13:53:20]

Done.

+const size_t kNumFramesToProcessStandalone = 50 * kNumFramesToProcess;
+const size_t kProcessingBatchSize = 300;
+const size_t kProcessingBatchSizeStandalone = 50 * kProcessingBatchSize;
+const size_t kWarmupBatchSize = 50 * kProcessingBatchSize;
+const size_t kWarmupBatchSizeStandalone = 50 * kWarmupBatchSize;
 const int kSampleRate = AudioProcessing::kSampleRate48kHz;
 const int kNumberOfChannels = 1;
 
@@ -43,16 +46,20 @@ void RunStandaloneSubmodule() {
   test::SimulatorBuffers buffers(
       kSampleRate, kSampleRate, kSampleRate, kSampleRate, kNumberOfChannels,
       kNumberOfChannels, kNumberOfChannels, kNumberOfChannels);
-  test::PerformanceTimer timer(kNumFramesToProcess);
+  test::PerformanceTimer timer(kNumFramesToProcessStandalone /

[hlundin-webrtc, 2017/03/17 08:48:03]

I suggest you modify PerformanceTimer to help you with the warm-up. You could
either set the warm-up number in the ctor and let it just ignore those first
samples, or add a method where you prune the first samples after the measurement
is done. I think I prefer the former, but can be persuaded otherwise...

[ivoc, 2017/03/17 13:53:20]

Good idea. I implemented it in a bit of a different way, by passing an argument
to the GetAverageDuration/GetStandardDeviation functions. That way we don't have
to store any extra state in the PerformanceTimer, I don't have to actually
delete any of the measurements stored in there and none of the other code that
uses it has to be modified. Is that okay?

[hlundin-webrtc, 2017/03/17 14:16:36]

Good solution!

+                               kProcessingBatchSizeStandalone);
 
   ResidualEchoDetector echo_detector;
   echo_detector.Initialize();
+  float sum = 0.f;
 
-  for (size_t frame_no = 0; frame_no < kNumFramesToProcess; ++frame_no) {
+  for (size_t frame_no = 0; frame_no < kNumFramesToProcessStandalone;
+       ++frame_no) {
     // The first batch of frames are for warming up, and are not part of the
     // benchmark. After that the processing time is measured in chunks of
     // kProcessingBatchSize frames.
-    if (frame_no >= kWarmupBatchSize && frame_no % kProcessingBatchSize == 0) {
+    if (frame_no >= kWarmupBatchSizeStandalone &&
+        frame_no % kProcessingBatchSizeStandalone == 0) {

[hlundin-webrtc, 2017/03/17 08:48:03]

I don't know if this will skew the measurement, but the modulo operator is
surprisingly expensive. You could for instance use the following instead:

// Before the for loop
size_t next_start_frame = kWarmupBatchSizeStandalone;

// In the for loop
if (frame_no == next_start_frame) {
  time.StartTimer();
  next_start_frame += kProcessingBatchSizeStandalone;
}

// ...

if (frame_no == next_start_frame - 1 && 
    frame_no > kWarmupBatchSizeStandalone) {
  timer.StopTimer();
}

Of course, if you follow my suggestion for adding the warm-up to the
PerformanceTimer, you won't have to worry about that here.

[ivoc, 2017/03/17 13:53:20]

Although I agree that modulo operations are pretty expensive (should be similar
to integer division), I don't think it should be very significant compared to
all of the calculations that are happening in the echo detector (which include
plenty of modulo operations, sqrts and divisions). 
Also, if the modulo operator is equally slow in each iteration, it will not
affect the usefulness of this perf test. 

[hlundin-webrtc, 2017/03/17 14:16:36]

Acknowledged.

       timer.StartTimer();
     }
 
@@ -63,12 +70,15 @@ void RunStandaloneSubmodule() {
     echo_detector.AnalyzeCaptureAudio(rtc::ArrayView<const float>(
         buffers.capture_input_buffer->split_bands_const_f(0)[kBand0To8kHz],
         buffers.capture_input_buffer->num_frames_per_band()));
+    sum += echo_detector.echo_likelihood();
 
-    if (frame_no >= kWarmupBatchSize &&
-        frame_no % kProcessingBatchSize == kProcessingBatchSize - 1) {
+    if (frame_no >= kWarmupBatchSizeStandalone &&
+        frame_no % kProcessingBatchSizeStandalone ==
+            kProcessingBatchSizeStandalone - 1) {
       timer.StopTimer();
     }
   }
+  EXPECT_EQ(0.0f, sum);

[hlundin-webrtc, 2017/03/17 08:48:03]

Do we know that this will be exactly 0.0? Are there any uncertainties that would
motivate using EXPECT_FLOAT_EQ instead?

[ivoc, 2017/03/17 13:53:20]

In this test both signals are filled with zeros, so a non-zero result would be
highly unexpected. The reason I added this is that I was afraid that if I don't
use the sum it may get optimized out which means the entire benchmark loop could
be optimized out.

[hlundin-webrtc, 2017/03/17 14:16:36]

Acknowledged.

   webrtc::test::PrintResultMeanAndError(
       "echo_detector_call_durations", "", "StandaloneEchoDetector",
       FormPerformanceMeasureString(timer), "us", false);
@@ -80,7 +90,7 @@ void RunTogetherWithApm(std::string test_description,
   test::SimulatorBuffers buffers(
       kSampleRate, kSampleRate, kSampleRate, kSampleRate, kNumberOfChannels,
       kNumberOfChannels, kNumberOfChannels, kNumberOfChannels);
-  test::PerformanceTimer timer(kNumFramesToProcess);
+  test::PerformanceTimer timer(kNumFramesToProcess / kProcessingBatchSize);
 
   webrtc::Config config;
   AudioProcessing::Config apm_config;