Switch critical section locks out for atomic operations

webrtc/modules/audio_processing/audio_processing_performance_unittest.cc

 /*
  *  Copyright (c) 2015 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 "webrtc/modules/audio_processing/audio_processing_impl.h"
 
 #include <math.h>
 
 #include <algorithm>
 #include <vector>
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webrtc/base/array_view.h"
-#include "webrtc/base/criticalsection.h"
+#include "webrtc/base/atomicops.h"
 #include "webrtc/base/platform_thread.h"
 #include "webrtc/base/random.h"
 #include "webrtc/base/safe_conversions.h"
 #include "webrtc/config.h"
 #include "webrtc/modules/audio_processing/test/test_utils.h"
 #include "webrtc/modules/include/module_common_types.h"
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/system_wrappers/include/event_wrapper.h"
 #include "webrtc/system_wrappers/include/sleep.h"
 #include "webrtc/test/testsupport/perf_test.h"
@@ skipping 131 matching lines @@
   }
 
   int sample_rate_hz = 16000;
   SettingsType simulation_settings = SettingsType::kDefaultApmDesktop;
 };
 
 // Handler for the frame counters.
 class FrameCounters {
  public:
   void IncreaseRenderCounter() {
-    rtc::CritScope cs(&crit_);
-    render_count_++;
+    rtc::AtomicOps::Increment(&render_count_);
   }
 
   void IncreaseCaptureCounter() {
-    rtc::CritScope cs(&crit_);
-    capture_count_++;
-  }
-
-  int GetCaptureCounter() const {
-    rtc::CritScope cs(&crit_);
-    return capture_count_;
-  }
-
-  int GetRenderCounter() const {
-    rtc::CritScope cs(&crit_);
-    return render_count_;
+    rtc::AtomicOps::Increment(&capture_count_);
   }
 
   int CaptureMinusRenderCounters() const {
-    rtc::CritScope cs(&crit_);
-    return capture_count_ - render_count_;
+    // The return value will be approximate, but that's good enough since
+    // by the time we return the value, it's not guaranteed to be correct
+    // anyway.
+    return rtc::AtomicOps::AcquireLoad(&capture_count_) -
+           rtc::AtomicOps::AcquireLoad(&render_count_);
   }
 
   int RenderMinusCaptureCounters() const {
     return -CaptureMinusRenderCounters();
   }
 
   bool BothCountersExceedeThreshold(int threshold) const {
-    rtc::CritScope cs(&crit_);
-    return (render_count_ > threshold && capture_count_ > threshold);
+    // TODO(tommi): We could use an event to signal this so that we don't need
+    // to be polling from the main thread and possibly steal cycles.
+    const int capture_count = rtc::AtomicOps::AcquireLoad(&capture_count_);
+    const int render_count = rtc::AtomicOps::AcquireLoad(&render_count_);
+    return (render_count > threshold && capture_count > threshold);
   }
 
  private:
-  rtc::CriticalSection crit_;
-  int render_count_ GUARDED_BY(crit_) = 0;
-  int capture_count_ GUARDED_BY(crit_) = 0;
+  int render_count_ = 0;
+  int capture_count_ = 0;
 };
 
-// Class that protects a flag using a lock.
+// Class that represents a flag that can only be raised.

[peah-webrtc, 2016/01/26 08:48:45]

This class should be replaced with an rtc:Event as that is exactly what it does.

 class LockedFlag {
  public:
   bool get_flag() const {
-    rtc::CritScope cs(&crit_);
-    return flag_;
+    return rtc::AtomicOps::AcquireLoad(&flag_);
   }
 
   void set_flag() {
-    rtc::CritScope cs(&crit_);
-    flag_ = true;
+    if (!get_flag())  // read-only operation to avoid affecting the cache-line.
+      rtc::AtomicOps::CompareAndSwap(&flag_, 0, 1);
   }
 
  private:
-  rtc::CriticalSection crit_;
-  bool flag_ GUARDED_BY(crit_) = false;
+  int flag_ = 0;
 };
 
 // Parent class for the thread processors.
 class TimedThreadApiProcessor {
  public:
   TimedThreadApiProcessor(ProcessorType processor_type,
                           Random* rand_gen,
                           FrameCounters* shared_counters_state,
                           LockedFlag* capture_call_checker,
                           CallSimulator* test_framework,
@@ skipping 179 matching lines @@
     frame_data_.output_stream_config.set_sample_rate_hz(
         simulation_config_->sample_rate_hz);
     frame_data_.output_stream_config.set_num_channels(1);
     frame_data_.output_stream_config.set_has_keyboard(false);
   }
 
   bool ReadyToProcess() {
     switch (processor_type_) {
       case ProcessorType::kRender:
         return ReadyToProcessRender();
-        break;
+
       case ProcessorType::kCapture:
         return ReadyToProcessCapture();
-        break;
     }
 
     // Should not be reached, but the return statement is needed for the code to
     // build successfully on Android.
     RTC_NOTREACHED();
     return false;
   }
 
   Random* rand_gen_ = nullptr;
   FrameCounters* frame_counters_ = nullptr;
@@ skipping 243 matching lines @@
   rtc::scoped_ptr<TimedThreadApiProcessor> capture_thread_state_;
 };
 
 // Implements the callback functionality for the threads.
 bool TimedThreadApiProcessor::Process() {
   PrepareFrame();
 
   // Wait in a spinlock manner until it is ok to start processing.
   // Note that SleepMs is not applicable since it only allows sleeping
   // on a millisecond basis which is too long.
+  // TODO(tommi): This loop may affect the performance of the test that it's
+  // meant to measure.  See if we could use events instead to signal readiness.

[peah-webrtc, 2016/01/26 08:48:45]

Yes, I think an event would be better to use here if there is a problem that one
thread is starving the other in the sense that the threads are not really
running in parallel.

   while (!ReadyToProcess()) {
   }
 
   int result = AudioProcessing::kNoError;
   switch (processor_type_) {
     case ProcessorType::kRender:
       result = ProcessRender();
       break;
     case ProcessorType::kCapture:
       result = ProcessCapture();
@@ skipping 13 matching lines @@
   // Run test and verify that it did not time out.
   EXPECT_EQ(kEventSignaled, Run());
 }
 
 INSTANTIATE_TEST_CASE_P(
     AudioProcessingPerformanceTest,
     CallSimulator,
     ::testing::ValuesIn(SimulationConfig::GenerateSimulationConfigs()));
 
 }  // namespace webrtc