- Add a SetPriority method to ThreadWrapper
- Remove 'priority' from CreateThread and related member variables from implementations
- Make supplying a name for threads, non-optional
BUG=
R=magjed@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/44729004
Cr-Commit-Position: refs/heads/master@{#8810}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8810 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/system_wrappers/interface/thread_wrapper.h b/webrtc/system_wrappers/interface/thread_wrapper.h
index 67ce866..7420561 100644
--- a/webrtc/system_wrappers/interface/thread_wrapper.h
+++ b/webrtc/system_wrappers/interface/thread_wrapper.h
@@ -16,6 +16,10 @@
#ifndef WEBRTC_SYSTEM_WRAPPERS_INTERFACE_THREAD_WRAPPER_H_
#define WEBRTC_SYSTEM_WRAPPERS_INTERFACE_THREAD_WRAPPER_H_
+#if defined(WEBRTC_WIN)
+#include <windows.h>
+#endif
+
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/common_types.h"
#include "webrtc/typedefs.h"
@@ -28,11 +32,19 @@
typedef bool(*ThreadRunFunction)(void*);
enum ThreadPriority {
+#ifdef WEBRTC_WIN
+ kLowPriority = THREAD_PRIORITY_BELOW_NORMAL,
+ kNormalPriority = THREAD_PRIORITY_NORMAL,
+ kHighPriority = THREAD_PRIORITY_ABOVE_NORMAL,
+ kHighestPriority = THREAD_PRIORITY_HIGHEST,
+ kRealtimePriority = THREAD_PRIORITY_TIME_CRITICAL
+#else
kLowPriority = 1,
kNormalPriority = 2,
kHighPriority = 3,
kHighestPriority = 4,
kRealtimePriority = 5
+#endif
};
// Represents a simple worker thread. The implementation must be assumed
@@ -52,11 +64,8 @@
// prio Thread priority. May require root/admin rights.
// thread_name NULL terminated thread name, will be visable in the Windows
// debugger.
- // TODO(tommi): Remove the priority argument and provide a setter instead.
- // TODO(tommi): Make thread_name non-optional (i.e. no default value).
static rtc::scoped_ptr<ThreadWrapper> CreateThread(ThreadRunFunction func,
- void* obj, ThreadPriority prio = kNormalPriority,
- const char* thread_name = 0);
+ void* obj, const char* thread_name);
// Get the current thread's thread ID.
// NOTE: This is a static method. It returns the id of the calling thread,
@@ -75,6 +84,10 @@
// Multiple tries to Stop are allowed (e.g. to wait longer than 2 seconds).
// It's ok to call Stop() even if the spawned thread has been reclaimed.
virtual bool Stop() = 0;
+
+ // Set the priority of the worker thread. Must be called when thread
+ // is running.
+ virtual bool SetPriority(ThreadPriority priority) = 0;
};
} // namespace webrtc
diff --git a/webrtc/system_wrappers/source/condition_variable_unittest.cc b/webrtc/system_wrappers/source/condition_variable_unittest.cc
index d6907f6..c34c4ea 100644
--- a/webrtc/system_wrappers/source/condition_variable_unittest.cc
+++ b/webrtc/system_wrappers/source/condition_variable_unittest.cc
@@ -145,7 +145,7 @@
virtual void SetUp() {
thread_ = ThreadWrapper::CreateThread(&WaitingRunFunction,
- &baton_);
+ &baton_, "CondVarTest");
ASSERT_TRUE(thread_->Start());
}
diff --git a/webrtc/system_wrappers/source/critical_section_unittest.cc b/webrtc/system_wrappers/source/critical_section_unittest.cc
index 9b497b4..ec639eb 100644
--- a/webrtc/system_wrappers/source/critical_section_unittest.cc
+++ b/webrtc/system_wrappers/source/critical_section_unittest.cc
@@ -79,7 +79,7 @@
CriticalSectionWrapper::CreateCriticalSection();
ProtectedCount count(crit_sect);
rtc::scoped_ptr<ThreadWrapper> thread = ThreadWrapper::CreateThread(
- &LockUnlockThenStopRunFunction, &count);
+ &LockUnlockThenStopRunFunction, &count, "ThreadWakesOnce");
crit_sect->Enter();
ASSERT_TRUE(thread->Start());
SwitchProcess();
@@ -106,7 +106,7 @@
CriticalSectionWrapper::CreateCriticalSection();
ProtectedCount count(crit_sect);
rtc::scoped_ptr<ThreadWrapper> thread = ThreadWrapper::CreateThread(
- &LockUnlockRunFunction, &count);
+ &LockUnlockRunFunction, &count, "ThreadWakesTwice");
crit_sect->Enter(); // Make sure counter stays 0 until we wait for it.
ASSERT_TRUE(thread->Start());
crit_sect->Leave();
diff --git a/webrtc/system_wrappers/source/data_log.cc b/webrtc/system_wrappers/source/data_log.cc
index 7c2a91b..653af65 100644
--- a/webrtc/system_wrappers/source/data_log.cc
+++ b/webrtc/system_wrappers/source/data_log.cc
@@ -349,13 +349,11 @@
int DataLogImpl::Init() {
file_writer_thread_ = ThreadWrapper::CreateThread(
- DataLogImpl::Run,
- instance_,
- kHighestPriority,
- "DataLog");
+ DataLogImpl::Run, instance_, "DataLog");
bool success = file_writer_thread_->Start();
if (!success)
return -1;
+ file_writer_thread_->SetPriority(kHighestPriority);
return 0;
}
diff --git a/webrtc/system_wrappers/source/event_posix.cc b/webrtc/system_wrappers/source/event_posix.cc
index bbfb0b0..6833e0e 100644
--- a/webrtc/system_wrappers/source/event_posix.cc
+++ b/webrtc/system_wrappers/source/event_posix.cc
@@ -154,11 +154,11 @@
// Start the timer thread
timer_event_ = static_cast<EventPosix*>(EventWrapper::Create());
const char* thread_name = "WebRtc_event_timer_thread";
- timer_thread_ = ThreadWrapper::CreateThread(Run, this, kRealtimePriority,
- thread_name);
+ timer_thread_ = ThreadWrapper::CreateThread(Run, this, thread_name);
periodic_ = periodic;
time_ = time;
bool started = timer_thread_->Start();
+ timer_thread_->SetPriority(kRealtimePriority);
pthread_mutex_unlock(&mutex_);
return started;
diff --git a/webrtc/system_wrappers/source/thread.cc b/webrtc/system_wrappers/source/thread.cc
index 957388b..b469344 100644
--- a/webrtc/system_wrappers/source/thread.cc
+++ b/webrtc/system_wrappers/source/thread.cc
@@ -25,10 +25,9 @@
#endif
rtc::scoped_ptr<ThreadWrapper> ThreadWrapper::CreateThread(
- ThreadRunFunction func, void* obj, ThreadPriority prio,
- const char* thread_name) {
+ ThreadRunFunction func, void* obj, const char* thread_name) {
return rtc::scoped_ptr<ThreadWrapper>(
- new ThreadType(func, obj, prio, thread_name)).Pass();
+ new ThreadType(func, obj, thread_name)).Pass();
}
} // namespace webrtc
diff --git a/webrtc/system_wrappers/source/thread_posix.cc b/webrtc/system_wrappers/source/thread_posix.cc
index c605411..dc95cbc 100644
--- a/webrtc/system_wrappers/source/thread_posix.cc
+++ b/webrtc/system_wrappers/source/thread_posix.cc
@@ -69,10 +69,9 @@
}
ThreadPosix::ThreadPosix(ThreadRunFunction func, void* obj,
- ThreadPriority prio, const char* thread_name)
+ const char* thread_name)
: run_function_(func),
obj_(obj),
- prio_(prio),
stop_event_(false, false),
name_(thread_name ? thread_name : "webrtc"),
thread_(0) {
@@ -112,6 +111,38 @@
return true;
}
+bool ThreadPosix::SetPriority(ThreadPriority priority) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ if (!thread_)
+ return false;
+
+#ifdef WEBRTC_THREAD_RR
+ const int policy = SCHED_RR;
+#else
+ const int policy = SCHED_FIFO;
+#endif
+ const int min_prio = sched_get_priority_min(policy);
+ const int max_prio = sched_get_priority_max(policy);
+ if (min_prio == -1 || max_prio == -1) {
+ WEBRTC_TRACE(kTraceError, kTraceUtility, -1,
+ "unable to retreive min or max priority for threads");
+ return false;
+ }
+
+ if (max_prio - min_prio <= 2)
+ return false;
+
+ sched_param param;
+ param.sched_priority = ConvertToSystemPriority(priority, min_prio, max_prio);
+ if (pthread_setschedparam(thread_, policy, ¶m) != 0) {
+ WEBRTC_TRACE(
+ kTraceError, kTraceUtility, -1, "unable to set thread priority");
+ return false;
+ }
+
+ return true;
+}
+
void ThreadPosix::Run() {
if (!name_.empty()) {
// Setting the thread name may fail (harmlessly) if running inside a
@@ -123,27 +154,6 @@
#endif
}
-#ifdef WEBRTC_THREAD_RR
- const int policy = SCHED_RR;
-#else
- const int policy = SCHED_FIFO;
-#endif
- const int min_prio = sched_get_priority_min(policy);
- const int max_prio = sched_get_priority_max(policy);
- if ((min_prio == -1) || (max_prio == -1)) {
- WEBRTC_TRACE(kTraceError, kTraceUtility, -1,
- "unable to retreive min or max priority for threads");
- }
-
- if (max_prio - min_prio > 2) {
- sched_param param;
- param.sched_priority = ConvertToSystemPriority(prio_, min_prio, max_prio);
- if (pthread_setschedparam(pthread_self(), policy, ¶m) != 0) {
- WEBRTC_TRACE(
- kTraceError, kTraceUtility, -1, "unable to set thread priority");
- }
- }
-
// It's a requirement that for successful thread creation that the run
// function be called at least once (see RunFunctionIsCalled unit test),
// so to fullfill that requirement, we use a |do| loop and not |while|.
diff --git a/webrtc/system_wrappers/source/thread_posix.h b/webrtc/system_wrappers/source/thread_posix.h
index ccc52b6..c726e48 100644
--- a/webrtc/system_wrappers/source/thread_posix.h
+++ b/webrtc/system_wrappers/source/thread_posix.h
@@ -25,14 +25,15 @@
class ThreadPosix : public ThreadWrapper {
public:
- ThreadPosix(ThreadRunFunction func, void* obj, ThreadPriority prio,
- const char* thread_name);
+ ThreadPosix(ThreadRunFunction func, void* obj, const char* thread_name);
~ThreadPosix() override;
// From ThreadWrapper.
bool Start() override;
bool Stop() override;
+ bool SetPriority(ThreadPriority priority) override;
+
private:
static void* StartThread(void* param);
@@ -41,7 +42,6 @@
rtc::ThreadChecker thread_checker_;
ThreadRunFunction const run_function_;
void* const obj_;
- ThreadPriority prio_;
rtc::Event stop_event_;
const std::string name_;
diff --git a/webrtc/system_wrappers/source/thread_unittest.cc b/webrtc/system_wrappers/source/thread_unittest.cc
index f5de7d2..854f98b 100644
--- a/webrtc/system_wrappers/source/thread_unittest.cc
+++ b/webrtc/system_wrappers/source/thread_unittest.cc
@@ -24,7 +24,7 @@
TEST(ThreadTest, StartStop) {
rtc::scoped_ptr<ThreadWrapper> thread = ThreadWrapper::CreateThread(
- &NullRunFunction, NULL);
+ &NullRunFunction, nullptr, "ThreadTest");
ASSERT_TRUE(thread->Start());
EXPECT_TRUE(thread->Stop());
}
@@ -40,7 +40,7 @@
TEST(ThreadTest, RunFunctionIsCalled) {
bool flag = false;
rtc::scoped_ptr<ThreadWrapper> thread = ThreadWrapper::CreateThread(
- &SetFlagRunFunction, &flag);
+ &SetFlagRunFunction, &flag, "RunFunctionIsCalled");
ASSERT_TRUE(thread->Start());
// At this point, the flag may be either true or false.
diff --git a/webrtc/system_wrappers/source/thread_win.cc b/webrtc/system_wrappers/source/thread_win.cc
index 0c1a8f2..aa03b91 100644
--- a/webrtc/system_wrappers/source/thread_win.cc
+++ b/webrtc/system_wrappers/source/thread_win.cc
@@ -55,10 +55,9 @@
}
ThreadWindows::ThreadWindows(ThreadRunFunction func, void* obj,
- ThreadPriority prio, const char* thread_name)
+ const char* thread_name)
: run_function_(func),
obj_(obj),
- prio_(prio),
stop_(false),
thread_(NULL),
name_(thread_name ? thread_name : "webrtc") {
@@ -98,28 +97,6 @@
return false;
}
- if (prio_ != kNormalPriority) {
- int priority = THREAD_PRIORITY_NORMAL;
- switch (prio_) {
- case kLowPriority:
- priority = THREAD_PRIORITY_BELOW_NORMAL;
- break;
- case kHighPriority:
- priority = THREAD_PRIORITY_ABOVE_NORMAL;
- break;
- case kHighestPriority:
- priority = THREAD_PRIORITY_HIGHEST;
- break;
- case kRealtimePriority:
- priority = THREAD_PRIORITY_TIME_CRITICAL;
- break;
- default:
- break;
- }
-
- SetThreadPriority(thread_, priority);
- }
-
return true;
}
@@ -136,6 +113,11 @@
return true;
}
+bool ThreadWindows::SetPriority(ThreadPriority priority) {
+ DCHECK(main_thread_.CalledOnValidThread());
+ return thread_ && SetThreadPriority(thread_, priority);
+}
+
void ThreadWindows::Run() {
if (!name_.empty())
SetThreadName(static_cast<DWORD>(-1), name_.c_str());
diff --git a/webrtc/system_wrappers/source/thread_win.h b/webrtc/system_wrappers/source/thread_win.h
index 1652551..741ae1e 100644
--- a/webrtc/system_wrappers/source/thread_win.h
+++ b/webrtc/system_wrappers/source/thread_win.h
@@ -21,13 +21,14 @@
class ThreadWindows : public ThreadWrapper {
public:
- ThreadWindows(ThreadRunFunction func, void* obj, ThreadPriority prio,
- const char* thread_name);
+ ThreadWindows(ThreadRunFunction func, void* obj, const char* thread_name);
~ThreadWindows() override;
bool Start() override;
bool Stop() override;
+ bool SetPriority(ThreadPriority priority) override;
+
protected:
void Run();
@@ -37,8 +38,6 @@
ThreadRunFunction const run_function_;
void* const obj_;
bool stop_;
- // TODO(tommi): Consider having a SetPriority method instead of this variable.
- ThreadPriority prio_;
HANDLE thread_;
const std::string name_;
rtc::ThreadChecker main_thread_;