Revert 8810 "- Add a SetPriority method to ThreadWrapper"

Seeing if this is causing roll issues.

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

TBR=tommi@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/48609004

Cr-Commit-Position: refs/heads/master@{#8818}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8818 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/system_wrappers/source/condition_variable_unittest.cc

@@ -145,7 +145,7 @@ class CondVarTest : public ::testing::Test {
 
   virtual void SetUp() {
     thread_ = ThreadWrapper::CreateThread(&WaitingRunFunction,
-                                          &baton_, "CondVarTest");
+                                          &baton_);
     ASSERT_TRUE(thread_->Start());
   }
 

webrtc/system_wrappers/source/critical_section_unittest.cc

@@ -79,7 +79,7 @@ TEST_F(CritSectTest, ThreadWakesOnce) NO_THREAD_SAFETY_ANALYSIS {
       CriticalSectionWrapper::CreateCriticalSection();
   ProtectedCount count(crit_sect);
   rtc::scoped_ptr<ThreadWrapper> thread = ThreadWrapper::CreateThread(
-      &LockUnlockThenStopRunFunction, &count, "ThreadWakesOnce");
+      &LockUnlockThenStopRunFunction, &count);
   crit_sect->Enter();
   ASSERT_TRUE(thread->Start());
   SwitchProcess();
@@ -106,7 +106,7 @@ TEST_F(CritSectTest, ThreadWakesTwice) NO_THREAD_SAFETY_ANALYSIS {
       CriticalSectionWrapper::CreateCriticalSection();
   ProtectedCount count(crit_sect);
   rtc::scoped_ptr<ThreadWrapper> thread = ThreadWrapper::CreateThread(
-      &LockUnlockRunFunction, &count, "ThreadWakesTwice");
+      &LockUnlockRunFunction, &count);
   crit_sect->Enter();  // Make sure counter stays 0 until we wait for it.
   ASSERT_TRUE(thread->Start());
   crit_sect->Leave();

webrtc/system_wrappers/source/data_log.cc

@@ -349,11 +349,13 @@ int DataLogImpl::CreateLog() {
 
 int DataLogImpl::Init() {
   file_writer_thread_ = ThreadWrapper::CreateThread(
-      DataLogImpl::Run, instance_, "DataLog");
+                          DataLogImpl::Run,
+                          instance_,
+                          kHighestPriority,
+                          "DataLog");
   bool success = file_writer_thread_->Start();
   if (!success)
     return -1;
-  file_writer_thread_->SetPriority(kHighestPriority);
   return 0;
 }
 

webrtc/system_wrappers/source/event_posix.cc

@@ -154,11 +154,11 @@ bool EventPosix::StartTimer(bool periodic, unsigned long time) {
   // 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, thread_name);
+  timer_thread_ = ThreadWrapper::CreateThread(Run, this, kRealtimePriority,
+                                              thread_name);
   periodic_ = periodic;
   time_ = time;
   bool started = timer_thread_->Start();
-  timer_thread_->SetPriority(kRealtimePriority);
   pthread_mutex_unlock(&mutex_);
 
   return started;

webrtc/system_wrappers/source/thread.cc

@@ -25,9 +25,10 @@ typedef ThreadPosix ThreadType;
 #endif
 
 rtc::scoped_ptr<ThreadWrapper> ThreadWrapper::CreateThread(
-    ThreadRunFunction func, void* obj, const char* thread_name) {
+    ThreadRunFunction func, void* obj, ThreadPriority prio,
+    const char* thread_name) {
   return rtc::scoped_ptr<ThreadWrapper>(
-      new ThreadType(func, obj, thread_name)).Pass();
+      new ThreadType(func, obj, prio, thread_name)).Pass();
 }
 
 }  // namespace webrtc

webrtc/system_wrappers/source/thread_posix.cc

@@ -69,9 +69,10 @@ void* ThreadPosix::StartThread(void* param) {
 }
 
 ThreadPosix::ThreadPosix(ThreadRunFunction func, void* obj,
-                         const char* thread_name)
+                         ThreadPriority prio, const char* thread_name)
     : run_function_(func),
       obj_(obj),
+      prio_(prio),
       stop_event_(false, false),
       name_(thread_name ? thread_name : "webrtc"),
       thread_(0) {
@@ -111,38 +112,6 @@ bool ThreadPosix::Stop() {
   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, &param) != 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
@@ -154,6 +123,27 @@ void ThreadPosix::Run() {
 #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, &param) != 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|.

webrtc/system_wrappers/source/thread_posix.h

@@ -25,15 +25,14 @@ int ConvertToSystemPriority(ThreadPriority priority, int min_prio,
 
 class ThreadPosix : public ThreadWrapper {
  public:
-  ThreadPosix(ThreadRunFunction func, void* obj, const char* thread_name);
+  ThreadPosix(ThreadRunFunction func, void* obj, ThreadPriority prio,
+              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);
 
@@ -42,6 +41,7 @@ class ThreadPosix : public ThreadWrapper {
   rtc::ThreadChecker thread_checker_;
   ThreadRunFunction const run_function_;
   void* const obj_;
+  ThreadPriority prio_;
   rtc::Event stop_event_;
   const std::string name_;
 

webrtc/system_wrappers/source/thread_unittest.cc

@@ -24,7 +24,7 @@ bool NullRunFunction(void* obj) {
 
 TEST(ThreadTest, StartStop) {
   rtc::scoped_ptr<ThreadWrapper> thread = ThreadWrapper::CreateThread(
-      &NullRunFunction, nullptr, "ThreadTest");
+      &NullRunFunction, NULL);
   ASSERT_TRUE(thread->Start());
   EXPECT_TRUE(thread->Stop());
 }
@@ -40,7 +40,7 @@ bool SetFlagRunFunction(void* obj) {
 TEST(ThreadTest, RunFunctionIsCalled) {
   bool flag = false;
   rtc::scoped_ptr<ThreadWrapper> thread = ThreadWrapper::CreateThread(
-      &SetFlagRunFunction, &flag, "RunFunctionIsCalled");
+      &SetFlagRunFunction, &flag);
   ASSERT_TRUE(thread->Start());
 
   // At this point, the flag may be either true or false.

webrtc/system_wrappers/source/thread_win.cc

@@ -55,9 +55,10 @@ void SetThreadName(DWORD dwThreadID, LPCSTR szThreadName) {
 }
 
 ThreadWindows::ThreadWindows(ThreadRunFunction func, void* obj,
-                             const char* thread_name)
+                             ThreadPriority prio, const char* thread_name)
     : run_function_(func),
       obj_(obj),
+      prio_(prio),
       stop_(false),
       thread_(NULL),
       name_(thread_name ? thread_name : "webrtc") {
@@ -97,6 +98,28 @@ bool ThreadWindows::Start() {
     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;
 }
 
@@ -113,11 +136,6 @@ bool ThreadWindows::Stop() {
   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());

webrtc/system_wrappers/source/thread_win.h

@@ -21,14 +21,13 @@ namespace webrtc {
 
 class ThreadWindows : public ThreadWrapper {
  public:
-  ThreadWindows(ThreadRunFunction func, void* obj, const char* thread_name);
+  ThreadWindows(ThreadRunFunction func, void* obj, ThreadPriority prio,
+                const char* thread_name);
   ~ThreadWindows() override;
 
   bool Start() override;
   bool Stop() override;
 
-  bool SetPriority(ThreadPriority priority) override;
-
  protected:
   void Run();
 
@@ -38,6 +37,8 @@ class ThreadWindows : public ThreadWrapper {
   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_;