Make CriticalSectionWrapper non-virtual.
There's no need for this class to have a vtable since there exists only a single implementation (per platform). It's also not good for performance.
BUG=
R=pbos@webrtc.org
Review URL: https://codereview.webrtc.org/1601743004 .
Cr-Original-Commit-Position: refs/heads/master@{#11306}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: ee5a309f12e4a3f96b8924d45e65ae01ef18d079
diff --git a/modules/audio_coding/neteq/neteq_network_stats_unittest.cc b/modules/audio_coding/neteq/neteq_network_stats_unittest.cc
index 34ca9ea..d7d48a3 100644
--- a/modules/audio_coding/neteq/neteq_network_stats_unittest.cc
+++ b/modules/audio_coding/neteq/neteq_network_stats_unittest.cc
@@ -85,10 +85,10 @@
static const int kMaxOutputSize = 960; // 10 ms * 48 kHz * 2 channels.
enum logic {
- IGNORE,
- EQUAL,
- SMALLER_THAN,
- LARGER_THAN,
+ kIgnore,
+ kEqual,
+ kSmallerThan,
+ kLargerThan,
};
struct NetEqNetworkStatsCheck {
@@ -143,13 +143,13 @@
#define CHECK_NETEQ_NETWORK_STATS(x)\
switch (expects.x) {\
- case EQUAL:\
+ case kEqual:\
EXPECT_EQ(stats.x, expects.stats_ref.x);\
break;\
- case SMALLER_THAN:\
+ case kSmallerThan:\
EXPECT_LT(stats.x, expects.stats_ref.x);\
break;\
- case LARGER_THAN:\
+ case kLargerThan:\
EXPECT_GT(stats.x, expects.stats_ref.x);\
break;\
default:\
@@ -204,18 +204,18 @@
void DecodeFecTest() {
external_decoder_->set_fec_enabled(false);
NetEqNetworkStatsCheck expects = {
- IGNORE, // current_buffer_size_ms
- IGNORE, // preferred_buffer_size_ms
- IGNORE, // jitter_peaks_found
- EQUAL, // packet_loss_rate
- EQUAL, // packet_discard_rate
- EQUAL, // expand_rate
- EQUAL, // voice_expand_rate
- IGNORE, // preemptive_rate
- EQUAL, // accelerate_rate
- EQUAL, // decoded_fec_rate
- IGNORE, // clockdrift_ppm
- EQUAL, // added_zero_samples
+ kIgnore, // current_buffer_size_ms
+ kIgnore, // preferred_buffer_size_ms
+ kIgnore, // jitter_peaks_found
+ kEqual, // packet_loss_rate
+ kEqual, // packet_discard_rate
+ kEqual, // expand_rate
+ kEqual, // voice_expand_rate
+ kIgnore, // preemptive_rate
+ kEqual, // accelerate_rate
+ kEqual, // decoded_fec_rate
+ kIgnore, // clockdrift_ppm
+ kEqual, // added_zero_samples
{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
};
RunTest(50, expects);
@@ -237,18 +237,18 @@
void NoiseExpansionTest() {
NetEqNetworkStatsCheck expects = {
- IGNORE, // current_buffer_size_ms
- IGNORE, // preferred_buffer_size_ms
- IGNORE, // jitter_peaks_found
- EQUAL, // packet_loss_rate
- EQUAL, // packet_discard_rate
- EQUAL, // expand_rate
- EQUAL, // speech_expand_rate
- IGNORE, // preemptive_rate
- EQUAL, // accelerate_rate
- EQUAL, // decoded_fec_rate
- IGNORE, // clockdrift_ppm
- EQUAL, // added_zero_samples
+ kIgnore, // current_buffer_size_ms
+ kIgnore, // preferred_buffer_size_ms
+ kIgnore, // jitter_peaks_found
+ kEqual, // packet_loss_rate
+ kEqual, // packet_discard_rate
+ kEqual, // expand_rate
+ kEqual, // speech_expand_rate
+ kIgnore, // preemptive_rate
+ kEqual, // accelerate_rate
+ kEqual, // decoded_fec_rate
+ kIgnore, // clockdrift_ppm
+ kEqual, // added_zero_samples
{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
};
RunTest(50, expects);
diff --git a/system_wrappers/BUILD.gn b/system_wrappers/BUILD.gn
index a73a32f..e6e930a 100644
--- a/system_wrappers/BUILD.gn
+++ b/system_wrappers/BUILD.gn
@@ -55,10 +55,6 @@
"source/cpu_features.cc",
"source/cpu_info.cc",
"source/critical_section.cc",
- "source/critical_section_posix.cc",
- "source/critical_section_posix.h",
- "source/critical_section_win.cc",
- "source/critical_section_win.h",
"source/data_log_c.cc",
"source/event.cc",
"source/event_timer_posix.cc",
diff --git a/system_wrappers/include/critical_section_wrapper.h b/system_wrappers/include/critical_section_wrapper.h
index 7dd217e..b9e05a3 100644
--- a/system_wrappers/include/critical_section_wrapper.h
+++ b/system_wrappers/include/critical_section_wrapper.h
@@ -14,23 +14,47 @@
// If the critical section is heavily contended it may be beneficial to use
// read/write locks instead.
+#if defined (WEBRTC_WIN)
+#include <windows.h>
+#else
+#include <pthread.h>
+#endif
+
#include "webrtc/base/thread_annotations.h"
#include "webrtc/common_types.h"
namespace webrtc {
+
class LOCKABLE CriticalSectionWrapper {
public:
- // Factory method, constructor disabled
+ // Legacy factory method, being deprecated. Please use the constructor.
+ // TODO(tommi): Remove the CriticalSectionWrapper class and move users over
+ // to using rtc::CriticalSection. Before we can do that though, we need to
+ // fix the problem with the ConditionVariable* classes (see below).
static CriticalSectionWrapper* CreateCriticalSection();
- virtual ~CriticalSectionWrapper() {}
+ CriticalSectionWrapper();
+ ~CriticalSectionWrapper();
// Tries to grab lock, beginning of a critical section. Will wait for the
// lock to become available if the grab failed.
- virtual void Enter() EXCLUSIVE_LOCK_FUNCTION() = 0;
+ void Enter() EXCLUSIVE_LOCK_FUNCTION();
// Returns a grabbed lock, end of critical section.
- virtual void Leave() UNLOCK_FUNCTION() = 0;
+ void Leave() UNLOCK_FUNCTION();
+
+private:
+#if defined (WEBRTC_WIN)
+ CRITICAL_SECTION crit_;
+
+ // TODO(tommi): Remove friendness.
+ friend class ConditionVariableEventWin;
+ friend class ConditionVariableNativeWin;
+#else
+ // TODO(tommi): Remove friendness.
+ pthread_mutex_t mutex_;
+ friend class ConditionVariablePosix;
+#endif
};
// RAII extension of the critical section. Prevents Enter/Leave mismatches and
diff --git a/system_wrappers/source/condition_variable_event_win.cc b/system_wrappers/source/condition_variable_event_win.cc
index 41b019d..f61e085 100644
--- a/system_wrappers/source/condition_variable_event_win.cc
+++ b/system_wrappers/source/condition_variable_event_win.cc
@@ -84,7 +84,8 @@
*/
#include "webrtc/system_wrappers/source/condition_variable_event_win.h"
-#include "webrtc/system_wrappers/source/critical_section_win.h"
+
+#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
namespace webrtc {
@@ -133,9 +134,7 @@
++(num_waiters_[eventID]);
LeaveCriticalSection(&num_waiters_crit_sect_);
- CriticalSectionWindows* cs =
- static_cast<CriticalSectionWindows*>(&crit_sect);
- LeaveCriticalSection(&cs->crit);
+ LeaveCriticalSection(&crit_sect.crit_);
HANDLE events[2];
events[0] = events_[WAKE];
events[1] = events_[eventID];
@@ -161,7 +160,7 @@
ResetEvent(events_[eventID]);
}
- EnterCriticalSection(&cs->crit);
+ EnterCriticalSection(&crit_sect.crit_);
return ret_val;
}
diff --git a/system_wrappers/source/condition_variable_native_win.cc b/system_wrappers/source/condition_variable_native_win.cc
index 45225f2..3c07230 100644
--- a/system_wrappers/source/condition_variable_native_win.cc
+++ b/system_wrappers/source/condition_variable_native_win.cc
@@ -8,9 +8,9 @@
* be found in the AUTHORS file in the root of the source tree.
*/
+#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
#include "webrtc/system_wrappers/include/trace.h"
#include "webrtc/system_wrappers/source/condition_variable_native_win.h"
-#include "webrtc/system_wrappers/source/critical_section_win.h"
namespace webrtc {
@@ -86,10 +86,8 @@
bool ConditionVariableNativeWin::SleepCS(CriticalSectionWrapper& crit_sect,
unsigned long max_time_in_ms) {
- CriticalSectionWindows* cs =
- static_cast<CriticalSectionWindows*>(&crit_sect);
BOOL ret_val = PSleepConditionVariableCS_(&condition_variable_,
- &(cs->crit), max_time_in_ms);
+ &crit_sect.crit_, max_time_in_ms);
return ret_val != 0;
}
diff --git a/system_wrappers/source/condition_variable_posix.cc b/system_wrappers/source/condition_variable_posix.cc
index b213042..c2de4e3 100644
--- a/system_wrappers/source/condition_variable_posix.cc
+++ b/system_wrappers/source/condition_variable_posix.cc
@@ -10,6 +10,8 @@
#include "webrtc/system_wrappers/source/condition_variable_posix.h"
+#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
+
#include <errno.h>
#if defined(WEBRTC_LINUX)
#include <time.h>
@@ -17,8 +19,6 @@
#include <sys/time.h>
#endif
-#include "webrtc/system_wrappers/source/critical_section_posix.h"
-
namespace webrtc {
ConditionVariableWrapper* ConditionVariablePosix::Create() {
@@ -70,9 +70,7 @@
}
void ConditionVariablePosix::SleepCS(CriticalSectionWrapper& crit_sect) {
- CriticalSectionPosix* cs = reinterpret_cast<CriticalSectionPosix*>(
- &crit_sect);
- pthread_cond_wait(&cond_, &cs->mutex_);
+ pthread_cond_wait(&cond_, &crit_sect.mutex_);
}
bool ConditionVariablePosix::SleepCS(CriticalSectionWrapper& crit_sect,
@@ -85,9 +83,6 @@
const int NANOSECONDS_PER_SECOND = 1000000000;
const int NANOSECONDS_PER_MILLISECOND = 1000000;
- CriticalSectionPosix* cs = reinterpret_cast<CriticalSectionPosix*>(
- &crit_sect);
-
if (max_time_inMS != INFINITE) {
timespec ts;
#ifndef WEBRTC_MAC
@@ -113,10 +108,10 @@
ts.tv_sec += ts.tv_nsec / NANOSECONDS_PER_SECOND;
ts.tv_nsec %= NANOSECONDS_PER_SECOND;
}
- const int res = pthread_cond_timedwait(&cond_, &cs->mutex_, &ts);
+ const int res = pthread_cond_timedwait(&cond_, &crit_sect.mutex_, &ts);
return (res == ETIMEDOUT) ? false : true;
} else {
- pthread_cond_wait(&cond_, &cs->mutex_);
+ pthread_cond_wait(&cond_, &crit_sect.mutex_);
return true;
}
}
diff --git a/system_wrappers/source/critical_section.cc b/system_wrappers/source/critical_section.cc
index c586588..a8a5a6d 100644
--- a/system_wrappers/source/critical_section.cc
+++ b/system_wrappers/source/critical_section.cc
@@ -8,21 +8,53 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#if defined(_WIN32)
-#include <windows.h>
-#include "webrtc/system_wrappers/source/critical_section_win.h"
-#else
-#include "webrtc/system_wrappers/source/critical_section_posix.h"
-#endif
+#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
namespace webrtc {
CriticalSectionWrapper* CriticalSectionWrapper::CreateCriticalSection() {
-#ifdef _WIN32
- return new CriticalSectionWindows();
-#else
- return new CriticalSectionPosix();
-#endif
+ return new CriticalSectionWrapper();
}
+#if defined (WEBRTC_WIN)
+
+CriticalSectionWrapper::CriticalSectionWrapper() {
+ InitializeCriticalSection(&crit_);
+}
+
+CriticalSectionWrapper::~CriticalSectionWrapper() {
+ DeleteCriticalSection(&crit_);
+}
+
+void CriticalSectionWrapper::Enter() {
+ EnterCriticalSection(&crit_);
+}
+
+void CriticalSectionWrapper::Leave() {
+ LeaveCriticalSection(&crit_);
+}
+
+#else
+
+CriticalSectionWrapper::CriticalSectionWrapper() {
+ pthread_mutexattr_t attr;
+ pthread_mutexattr_init(&attr);
+ pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
+ pthread_mutex_init(&mutex_, &attr);
+}
+
+CriticalSectionWrapper::~CriticalSectionWrapper() {
+ pthread_mutex_destroy(&mutex_);
+}
+
+void CriticalSectionWrapper::Enter() {
+ pthread_mutex_lock(&mutex_);
+}
+
+void CriticalSectionWrapper::Leave() {
+ pthread_mutex_unlock(&mutex_);
+}
+
+#endif
+
} // namespace webrtc
diff --git a/system_wrappers/source/critical_section_posix.cc b/system_wrappers/source/critical_section_posix.cc
deleted file mode 100644
index 41b7732..0000000
--- a/system_wrappers/source/critical_section_posix.cc
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Copyright (c) 2012 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.
- */
-
-// General note: return values for the various pthread synchronization APIs
-// are explicitly ignored here. In Chromium, the same thing is done for release.
-// However, in debugging, failure in these APIs are logged.
-// TODO(henrike): add logging when pthread synchronization APIs are failing.
-
-#include "webrtc/system_wrappers/source/critical_section_posix.h"
-
-namespace webrtc {
-
-CriticalSectionPosix::CriticalSectionPosix() {
- pthread_mutexattr_t attr;
- (void) pthread_mutexattr_init(&attr);
- (void) pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
- (void) pthread_mutex_init(&mutex_, &attr);
-}
-
-CriticalSectionPosix::~CriticalSectionPosix() {
- (void) pthread_mutex_destroy(&mutex_);
-}
-
-void
-CriticalSectionPosix::Enter() {
- (void) pthread_mutex_lock(&mutex_);
-}
-
-void
-CriticalSectionPosix::Leave() {
- (void) pthread_mutex_unlock(&mutex_);
-}
-
-} // namespace webrtc
diff --git a/system_wrappers/source/critical_section_posix.h b/system_wrappers/source/critical_section_posix.h
deleted file mode 100644
index 099f74c..0000000
--- a/system_wrappers/source/critical_section_posix.h
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * Copyright (c) 2011 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.
- */
-
-#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_CRITICAL_SECTION_POSIX_H_
-#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_CRITICAL_SECTION_POSIX_H_
-
-#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
-
-#include <pthread.h>
-
-namespace webrtc {
-
-class CriticalSectionPosix : public CriticalSectionWrapper {
- public:
- CriticalSectionPosix();
-
- ~CriticalSectionPosix() override;
-
- void Enter() override;
- void Leave() override;
-
- private:
- pthread_mutex_t mutex_;
- friend class ConditionVariablePosix;
-};
-
-} // namespace webrtc
-
-#endif // WEBRTC_SYSTEM_WRAPPERS_SOURCE_CRITICAL_SECTION_POSIX_H_
diff --git a/system_wrappers/source/critical_section_win.cc b/system_wrappers/source/critical_section_win.cc
deleted file mode 100644
index b5149d1..0000000
--- a/system_wrappers/source/critical_section_win.cc
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Copyright (c) 2011 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/system_wrappers/source/critical_section_win.h"
-
-namespace webrtc {
-
-CriticalSectionWindows::CriticalSectionWindows() {
- InitializeCriticalSection(&crit);
-}
-
-CriticalSectionWindows::~CriticalSectionWindows() {
- DeleteCriticalSection(&crit);
-}
-
-void
-CriticalSectionWindows::Enter() {
- EnterCriticalSection(&crit);
-}
-
-void
-CriticalSectionWindows::Leave() {
- LeaveCriticalSection(&crit);
-}
-
-} // namespace webrtc
diff --git a/system_wrappers/source/critical_section_win.h b/system_wrappers/source/critical_section_win.h
deleted file mode 100644
index 8268bc3..0000000
--- a/system_wrappers/source/critical_section_win.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * Copyright (c) 2011 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.
- */
-
-#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_CRITICAL_SECTION_WIN_H_
-#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_CRITICAL_SECTION_WIN_H_
-
-#include <windows.h>
-#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
-#include "webrtc/typedefs.h"
-
-namespace webrtc {
-
-class CriticalSectionWindows : public CriticalSectionWrapper {
- public:
- CriticalSectionWindows();
-
- virtual ~CriticalSectionWindows();
-
- virtual void Enter();
- virtual void Leave();
-
- private:
- CRITICAL_SECTION crit;
-
- friend class ConditionVariableEventWin;
- friend class ConditionVariableNativeWin;
-};
-
-} // namespace webrtc
-
-#endif // WEBRTC_SYSTEM_WRAPPERS_SOURCE_CRITICAL_SECTION_WIN_H_
diff --git a/system_wrappers/source/rw_lock_winxp_win.h b/system_wrappers/source/rw_lock_winxp_win.h
index 20e5382..35decca 100644
--- a/system_wrappers/source/rw_lock_winxp_win.h
+++ b/system_wrappers/source/rw_lock_winxp_win.h
@@ -13,7 +13,7 @@
#include "webrtc/system_wrappers/include/rw_lock_wrapper.h"
#include "webrtc/system_wrappers/source/condition_variable_event_win.h"
-#include "webrtc/system_wrappers/source/critical_section_win.h"
+#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
#include "webrtc/typedefs.h"
namespace webrtc {
@@ -30,7 +30,7 @@
void ReleaseLockShared() override;
private:
- CriticalSectionWindows critical_section_;
+ CriticalSectionWrapper critical_section_;
ConditionVariableEventWin read_condition_;
ConditionVariableEventWin write_condition_;
diff --git a/system_wrappers/system_wrappers.gyp b/system_wrappers/system_wrappers.gyp
index f294c9e..479c02a 100644
--- a/system_wrappers/system_wrappers.gyp
+++ b/system_wrappers/system_wrappers.gyp
@@ -64,10 +64,6 @@
'source/cpu_info.cc',
'source/cpu_features.cc',
'source/critical_section.cc',
- 'source/critical_section_posix.cc',
- 'source/critical_section_posix.h',
- 'source/critical_section_win.cc',
- 'source/critical_section_win.h',
'source/data_log.cc',
'source/data_log_c.cc',
'source/data_log_no_op.cc',
