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-Commit-Position: refs/heads/master@{#11306}
This commit is contained in:
Tommi 2016-01-19 15:42:45 +01:00
parent dd45eb6801
commit ee5a309f12
13 changed files with 115 additions and 223 deletions

View File

@ -85,10 +85,10 @@ class NetEqNetworkStatsTest : public NetEqExternalDecoderTest {
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 @@ struct NetEqNetworkStatsCheck {
#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 @@ struct NetEqNetworkStatsCheck {
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 @@ struct NetEqNetworkStatsCheck {
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);

View File

@ -55,10 +55,6 @@ static_library("system_wrappers") {
"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",

View File

@ -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

View File

@ -84,7 +84,8 @@ Vanderbilt University to appear in their names.
*/
#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 @@ bool ConditionVariableEventWin::SleepCS(CriticalSectionWrapper& crit_sect,
++(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 @@ bool ConditionVariableEventWin::SleepCS(CriticalSectionWrapper& crit_sect,
ResetEvent(events_[eventID]);
}
EnterCriticalSection(&cs->crit);
EnterCriticalSection(&crit_sect.crit_);
return ret_val;
}

View File

@ -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 @@ void ConditionVariableNativeWin::SleepCS(CriticalSectionWrapper& crit_sect) {
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;
}

View File

@ -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 @@ ConditionVariablePosix::~ConditionVariablePosix() {
}
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 @@ bool ConditionVariablePosix::SleepCS(CriticalSectionWrapper& crit_sect,
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 @@ bool ConditionVariablePosix::SleepCS(CriticalSectionWrapper& crit_sect,
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;
}
}

View File

@ -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

View File

@ -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

View File

@ -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_

View File

@ -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

View File

@ -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_

View File

@ -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 @@ class RWLockWinXP : public RWLockWrapper {
void ReleaseLockShared() override;
private:
CriticalSectionWindows critical_section_;
CriticalSectionWrapper critical_section_;
ConditionVariableEventWin read_condition_;
ConditionVariableEventWin write_condition_;

View File

@ -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',