Remove thread id from ThreadWrapper::Start().
Removes ThreadPosix::InitParams and a corresponding wait for an event.
This unblocks ThreadPosix::Start which had to wait for thread scheduling
for an event to trigger on the spawned thread, giving faster Start()
calls.
BUG=4413
R=tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/43699004
Cr-Commit-Position: refs/heads/master@{#8709}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8709 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/system_wrappers/source/condition_variable_unittest.cc b/webrtc/system_wrappers/source/condition_variable_unittest.cc
index 9b8a6dd..9ca4ca9 100644
--- a/webrtc/system_wrappers/source/condition_variable_unittest.cc
+++ b/webrtc/system_wrappers/source/condition_variable_unittest.cc
@@ -146,8 +146,7 @@
virtual void SetUp() {
thread_ = ThreadWrapper::CreateThread(&WaitingRunFunction,
&baton_);
- unsigned int id = 42;
- ASSERT_TRUE(thread_->Start(id));
+ ASSERT_TRUE(thread_->Start());
}
virtual void TearDown() {
diff --git a/webrtc/system_wrappers/source/critical_section_unittest.cc b/webrtc/system_wrappers/source/critical_section_unittest.cc
index d876b13..b291d39 100644
--- a/webrtc/system_wrappers/source/critical_section_unittest.cc
+++ b/webrtc/system_wrappers/source/critical_section_unittest.cc
@@ -80,9 +80,8 @@
ProtectedCount count(crit_sect);
ThreadWrapper* thread = ThreadWrapper::CreateThread(
&LockUnlockThenStopRunFunction, &count);
- unsigned int id = 42;
crit_sect->Enter();
- ASSERT_TRUE(thread->Start(id));
+ ASSERT_TRUE(thread->Start());
SwitchProcess();
// The critical section is of reentrant mode, so this should not release
// the lock, even though count.Count() locks and unlocks the critical section
@@ -109,9 +108,8 @@
ProtectedCount count(crit_sect);
ThreadWrapper* thread = ThreadWrapper::CreateThread(&LockUnlockRunFunction,
&count);
- unsigned int id = 42;
crit_sect->Enter(); // Make sure counter stays 0 until we wait for it.
- ASSERT_TRUE(thread->Start(id));
+ ASSERT_TRUE(thread->Start());
crit_sect->Leave();
// The thread is capable of grabbing the lock multiple times,
diff --git a/webrtc/system_wrappers/source/data_log.cc b/webrtc/system_wrappers/source/data_log.cc
index bf6b7b3..41da872 100644
--- a/webrtc/system_wrappers/source/data_log.cc
+++ b/webrtc/system_wrappers/source/data_log.cc
@@ -358,8 +358,7 @@
"DataLog");
if (file_writer_thread_ == NULL)
return -1;
- unsigned int thread_id = 0;
- bool success = file_writer_thread_->Start(thread_id);
+ bool success = file_writer_thread_->Start();
if (!success)
return -1;
return 0;
diff --git a/webrtc/system_wrappers/source/event_posix.cc b/webrtc/system_wrappers/source/event_posix.cc
index ef85b01..c873e3a 100644
--- a/webrtc/system_wrappers/source/event_posix.cc
+++ b/webrtc/system_wrappers/source/event_posix.cc
@@ -205,8 +205,7 @@
thread_name);
periodic_ = periodic;
time_ = time;
- unsigned int id = 0;
- bool started = timer_thread_->Start(id);
+ bool started = timer_thread_->Start();
pthread_mutex_unlock(&mutex_);
return started;
diff --git a/webrtc/system_wrappers/source/thread_posix.cc b/webrtc/system_wrappers/source/thread_posix.cc
index fe928ef..84ed52d 100644
--- a/webrtc/system_wrappers/source/thread_posix.cc
+++ b/webrtc/system_wrappers/source/thread_posix.cc
@@ -62,18 +62,8 @@
return low_prio;
}
-struct ThreadPosix::InitParams {
- InitParams(ThreadPosix* thread)
- : me(thread), started(EventWrapper::Create()) {
- }
- ThreadPosix* me;
- rtc::scoped_ptr<EventWrapper> started;
-};
-
-// static
void* ThreadPosix::StartThread(void* param) {
- auto params = static_cast<InitParams*>(param);
- params->me->Run(params);
+ static_cast<ThreadPosix*>(param)->Run();
return 0;
}
@@ -82,9 +72,9 @@
: run_function_(func),
obj_(obj),
prio_(prio),
+ started_(false),
stop_event_(true, false),
name_(thread_name ? thread_name : "webrtc"),
- thread_id_(0),
thread_(0) {
DCHECK(name_.length() < kThreadMaxNameLength);
}
@@ -97,44 +87,34 @@
DCHECK(thread_checker_.CalledOnValidThread());
}
-bool ThreadPosix::Start(unsigned int& thread_id) {
+// TODO(pbos): Make Start void, calling code really doesn't support failures
+// here.
+bool ThreadPosix::Start() {
DCHECK(thread_checker_.CalledOnValidThread());
- DCHECK(!thread_id_) << "Thread already started?";
ThreadAttributes attr;
// Set the stack stack size to 1M.
pthread_attr_setstacksize(&attr, 1024 * 1024);
- InitParams params(this);
- int result = pthread_create(&thread_, &attr, &StartThread, ¶ms);
- if (result != 0)
- return false;
-
- CHECK_EQ(kEventSignaled, params.started->Wait(WEBRTC_EVENT_INFINITE));
- DCHECK_NE(thread_id_, 0);
-
- thread_id = thread_id_;
-
+ CHECK_EQ(0, pthread_create(&thread_, &attr, &StartThread, this));
+ started_ = true;
return true;
}
bool ThreadPosix::Stop() {
DCHECK(thread_checker_.CalledOnValidThread());
- if (!thread_id_)
+ if (!started_)
return true;
stop_event_.Set();
CHECK_EQ(0, pthread_join(thread_, nullptr));
- thread_id_ = 0;
+ started_ = false;
stop_event_.Reset();
return true;
}
-void ThreadPosix::Run(ThreadPosix::InitParams* params) {
- thread_id_ = rtc::CurrentThreadId();
- params->started->Set();
-
+void ThreadPosix::Run() {
if (!name_.empty()) {
// Setting the thread name may fail (harmlessly) if running inside a
// sandbox. Ignore failures if they happen.
diff --git a/webrtc/system_wrappers/source/thread_posix.h b/webrtc/system_wrappers/source/thread_posix.h
index ec9c10e..c1cf8b5 100644
--- a/webrtc/system_wrappers/source/thread_posix.h
+++ b/webrtc/system_wrappers/source/thread_posix.h
@@ -30,23 +30,22 @@
~ThreadPosix() override;
// From ThreadWrapper.
- bool Start(unsigned int& id) override;
+ bool Start() override;
bool Stop() override;
private:
static void* StartThread(void* param);
- struct InitParams;
- void Run(InitParams* params);
+ void Run();
rtc::ThreadChecker thread_checker_;
ThreadRunFunction const run_function_;
void* const obj_;
ThreadPriority prio_;
+ bool started_;
rtc::Event stop_event_;
const std::string name_;
- pid_t thread_id_;
pthread_t thread_;
};
diff --git a/webrtc/system_wrappers/source/thread_unittest.cc b/webrtc/system_wrappers/source/thread_unittest.cc
index a2228e6..99832d4 100644
--- a/webrtc/system_wrappers/source/thread_unittest.cc
+++ b/webrtc/system_wrappers/source/thread_unittest.cc
@@ -24,8 +24,7 @@
TEST(ThreadTest, StartStop) {
ThreadWrapper* thread = ThreadWrapper::CreateThread(&NullRunFunction, NULL);
- unsigned int id = 42;
- ASSERT_TRUE(thread->Start(id));
+ ASSERT_TRUE(thread->Start());
EXPECT_TRUE(thread->Stop());
delete thread;
}
@@ -42,8 +41,7 @@
bool flag = false;
ThreadWrapper* thread = ThreadWrapper::CreateThread(&SetFlagRunFunction,
&flag);
- unsigned int id = 42;
- ASSERT_TRUE(thread->Start(id));
+ ASSERT_TRUE(thread->Start());
// At this point, the flag may be either true or false.
EXPECT_TRUE(thread->Stop());
diff --git a/webrtc/system_wrappers/source/thread_win.cc b/webrtc/system_wrappers/source/thread_win.cc
index 67312fa..716434a 100644
--- a/webrtc/system_wrappers/source/thread_win.cc
+++ b/webrtc/system_wrappers/source/thread_win.cc
@@ -81,7 +81,7 @@
return 0;
}
-bool ThreadWindows::Start(unsigned int& id) {
+bool ThreadWindows::Start() {
DCHECK(main_thread_.CalledOnValidThread());
DCHECK(!thread_);
@@ -98,8 +98,6 @@
return false;
}
- id = thread_id;
-
if (prio_ != kNormalPriority) {
int priority = THREAD_PRIORITY_NORMAL;
switch (prio_) {
diff --git a/webrtc/system_wrappers/source/thread_win.h b/webrtc/system_wrappers/source/thread_win.h
index 0da8a53..1652551 100644
--- a/webrtc/system_wrappers/source/thread_win.h
+++ b/webrtc/system_wrappers/source/thread_win.h
@@ -25,7 +25,7 @@
const char* thread_name);
~ThreadWindows() override;
- bool Start(unsigned int& id) override;
+ bool Start() override;
bool Stop() override;
protected: