Improve stability of the echo detector complexity perf tests.

webrtc/modules/audio_processing/residual_echo_detector_complexity_unittest.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include <numeric>
 #include <vector>
 
 #include "webrtc/base/array_view.h"
 #include "webrtc/base/random.h"
 #include "webrtc/modules/audio_processing/audio_buffer.h"
 #include "webrtc/modules/audio_processing/include/audio_processing.h"
 #include "webrtc/modules/audio_processing/residual_echo_detector.h"
 #include "webrtc/modules/audio_processing/test/audio_buffer_tools.h"
 #include "webrtc/modules/audio_processing/test/performance_timer.h"
 #include "webrtc/modules/audio_processing/test/simulator_buffers.h"
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/test/gtest.h"
 #include "webrtc/test/testsupport/perf_test.h"
 
 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;
 
 std::string FormPerformanceMeasureString(const test::PerformanceTimer& timer) {
   std::string s = std::to_string(timer.GetDurationAverage());
   s += ", ";
   s += std::to_string(timer.GetDurationStandardDeviation());
   return s;
 }
 
 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();
     }
 
     buffers.UpdateInputBuffers();
     echo_detector.AnalyzeRenderAudio(rtc::ArrayView<const float>(
         buffers.render_input_buffer->split_bands_const_f(0)[kBand0To8kHz],
         buffers.render_input_buffer->num_frames_per_band()));
     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);
 }
 
 void RunTogetherWithApm(std::string test_description,
                         bool use_mobile_aec,
                         bool include_default_apm_processing) {
   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;
   if (include_default_apm_processing) {
     config.Set<DelayAgnostic>(new DelayAgnostic(true));
     config.Set<ExtendedFilter>(new ExtendedFilter(true));
   }
   apm_config.level_controller.enabled = include_default_apm_processing;
   apm_config.residual_echo_detector.enabled = true;
 
@@ skipping 72 matching lines @@
 TEST(EchoDetectorPerformanceTest, ProcessingViaApm) {
   RunTogetherWithApm("SimpleEchoDetectorViaApm", false, false);
 }
 
 TEST(EchoDetectorPerformanceTest, InteractionWithDefaultApm) {
   RunTogetherWithApm("EchoDetectorAndDefaultDesktopApm", false, true);
   RunTogetherWithApm("EchoDetectorAndDefaultMobileApm", true, true);
 }
 
 }  // namespace webrtc