Introduce a new constructor to PlatformThread.
The new constructor introduces two new changes:
* Support specifying thread priority at construction time.
- Moving forward, the SetPriority() method will be removed.
* New thread function type.
- The new type has 'void' as a return type and a polling loop
inside PlatformThread, is not used.
The old function type is still supported until all places have been moved over.
In this CL, the first steps towards deprecating the old mechanism are taken
by moving parts of the code that were simple to move, over to the new callback
type.
BUG=webrtc:7187
Review-Url: https://codereview.webrtc.org/2708723003
Cr-Commit-Position: refs/heads/master@{#16779}
diff --git a/webrtc/base/event_tracer.cc b/webrtc/base/event_tracer.cc
index 4393bff..cb7554a 100644
--- a/webrtc/base/event_tracer.cc
+++ b/webrtc/base/event_tracer.cc
@@ -80,7 +80,7 @@
namespace tracing {
namespace {
-static bool EventTracingThreadFunc(void* params);
+static void EventTracingThreadFunc(void* params);
// Atomic-int fast path for avoiding logging when disabled.
static volatile int g_event_logging_active = 0;
@@ -89,7 +89,10 @@
class EventLogger final {
public:
EventLogger()
- : logging_thread_(EventTracingThreadFunc, this, "EventTracingThread"),
+ : logging_thread_(EventTracingThreadFunc,
+ this,
+ "EventTracingThread",
+ kLowPriority),
shutdown_event_(false, false) {}
~EventLogger() { RTC_DCHECK(thread_checker_.CalledOnValidThread()); }
@@ -210,7 +213,6 @@
// Finally start, everything should be set up now.
logging_thread_.Start();
TRACE_EVENT_INSTANT0("webrtc", "EventLogger::Start");
- logging_thread_.SetPriority(kLowPriority);
}
void Stop() {
@@ -326,11 +328,8 @@
bool output_file_owned_ = false;
};
-static bool EventTracingThreadFunc(void* params) {
+static void EventTracingThreadFunc(void* params) {
static_cast<EventLogger*>(params)->Log();
- // False indicates that the thread function has done its job and doesn't need
- // to be restarted again. Log() runs its own internal loop.
- return false;
}
static EventLogger* volatile g_event_logger = nullptr;
diff --git a/webrtc/base/platform_thread.cc b/webrtc/base/platform_thread.cc
index d1bd509..525f0dd 100644
--- a/webrtc/base/platform_thread.cc
+++ b/webrtc/base/platform_thread.cc
@@ -92,14 +92,27 @@
#endif // defined(WEBRTC_WIN)
}
-PlatformThread::PlatformThread(ThreadRunFunction func,
+PlatformThread::PlatformThread(ThreadRunFunctionDeprecated func,
void* obj,
const char* thread_name)
- : run_function_(func),
+ : run_function_deprecated_(func),
obj_(obj),
name_(thread_name ? thread_name : "webrtc") {
RTC_DCHECK(func);
RTC_DCHECK(name_.length() < 64);
+ spawned_thread_checker_.DetachFromThread();
+}
+
+PlatformThread::PlatformThread(ThreadRunFunction func,
+ void* obj,
+ const char* thread_name,
+ ThreadPriority priority /*= kNormalPriority*/)
+ : run_function_(func), priority_(priority), obj_(obj), name_(thread_name) {
+ RTC_DCHECK(func);
+ RTC_DCHECK(!name_.empty());
+ // TODO(tommi): Consider lowering the limit to 15 (limit on Linux).
+ RTC_DCHECK(name_.length() < 64);
+ spawned_thread_checker_.DetachFromThread();
}
PlatformThread::~PlatformThread() {
@@ -180,11 +193,14 @@
thread_ = nullptr;
thread_id_ = 0;
#else
- RTC_CHECK_EQ(1, AtomicOps::Increment(&stop_flag_));
+ if (!run_function_)
+ RTC_CHECK_EQ(1, AtomicOps::Increment(&stop_flag_));
RTC_CHECK_EQ(0, pthread_join(thread_, nullptr));
- AtomicOps::ReleaseStore(&stop_flag_, 0);
+ if (!run_function_)
+ AtomicOps::ReleaseStore(&stop_flag_, 0);
thread_ = 0;
#endif // defined(WEBRTC_WIN)
+ spawned_thread_checker_.DetachFromThread();
}
// TODO(tommi): Deprecate the loop behavior in PlatformThread.
@@ -195,8 +211,16 @@
// All implementations will need to be aware of how the thread should be stopped
// and encouraging a busy polling loop, can be costly in terms of power and cpu.
void PlatformThread::Run() {
- if (!name_.empty())
- rtc::SetCurrentThreadName(name_.c_str());
+ // Attach the worker thread checker to this thread.
+ RTC_DCHECK(spawned_thread_checker_.CalledOnValidThread());
+ rtc::SetCurrentThreadName(name_.c_str());
+
+ if (run_function_) {
+ SetPriority(priority_);
+ run_function_(obj_);
+ return;
+ }
+// TODO(tommi): Delete the below.
#if !defined(WEBRTC_MAC) && !defined(WEBRTC_WIN)
const struct timespec ts_null = {0};
#endif
@@ -204,7 +228,7 @@
// The interface contract of Start/Stop is that for a successful call to
// Start, there should be at least one call to the run function. So we
// call the function before checking |stop_|.
- if (!run_function_(obj_))
+ if (!run_function_deprecated_(obj_))
break;
#if defined(WEBRTC_WIN)
// Alertable sleep to permit RaiseFlag to run and update |stop_|.
@@ -221,8 +245,19 @@
}
bool PlatformThread::SetPriority(ThreadPriority priority) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
- RTC_DCHECK(IsRunning());
+#if RTC_DCHECK_IS_ON
+ if (run_function_) {
+ // The non-deprecated way of how this function gets called, is that it must
+ // be called on the worker thread itself.
+ RTC_DCHECK(spawned_thread_checker_.CalledOnValidThread());
+ } else {
+ // In the case of deprecated use of this method, it must be called on the
+ // same thread as the PlatformThread object is constructed on.
+ RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(IsRunning());
+ }
+#endif
+
#if defined(WEBRTC_WIN)
return SetThreadPriority(thread_, priority) != FALSE;
#elif defined(__native_client__)
diff --git a/webrtc/base/platform_thread.h b/webrtc/base/platform_thread.h
index 667407f..ed26ca0 100644
--- a/webrtc/base/platform_thread.h
+++ b/webrtc/base/platform_thread.h
@@ -32,7 +32,8 @@
// Callback function that the spawned thread will enter once spawned.
// A return value of false is interpreted as that the function has no
// more work to do and that the thread can be released.
-typedef bool (*ThreadRunFunction)(void*);
+typedef bool (*ThreadRunFunctionDeprecated)(void*);
+typedef void (*ThreadRunFunction)(void*);
enum ThreadPriority {
#ifdef WEBRTC_WIN
@@ -55,7 +56,13 @@
// called from the same thread, including instantiation.
class PlatformThread {
public:
- PlatformThread(ThreadRunFunction func, void* obj, const char* thread_name);
+ PlatformThread(ThreadRunFunctionDeprecated func,
+ void* obj,
+ const char* thread_name);
+ PlatformThread(ThreadRunFunction func,
+ void* obj,
+ const char* thread_name,
+ ThreadPriority priority = kNormalPriority);
virtual ~PlatformThread();
const std::string& name() const { return name_; }
@@ -74,6 +81,7 @@
void Stop();
// Set the priority of the thread. Must be called when thread is running.
+ // TODO(tommi): Make private and only allow public support via ctor.
bool SetPriority(ThreadPriority priority);
protected:
@@ -85,12 +93,15 @@
private:
void Run();
- ThreadRunFunction const run_function_;
+ ThreadRunFunctionDeprecated const run_function_deprecated_ = nullptr;
+ ThreadRunFunction const run_function_ = nullptr;
+ const ThreadPriority priority_ = kNormalPriority;
void* const obj_;
// TODO(pbos): Make sure call sites use string literals and update to a const
// char* instead of a std::string.
const std::string name_;
rtc::ThreadChecker thread_checker_;
+ rtc::ThreadChecker spawned_thread_checker_;
#if defined(WEBRTC_WIN)
static DWORD WINAPI StartThread(void* param);
diff --git a/webrtc/base/platform_thread_unittest.cc b/webrtc/base/platform_thread_unittest.cc
index 322eb35..d6d35e4 100644
--- a/webrtc/base/platform_thread_unittest.cc
+++ b/webrtc/base/platform_thread_unittest.cc
@@ -16,20 +16,65 @@
namespace rtc {
namespace {
// Function that does nothing, and reports success.
-bool NullRunFunction(void* obj) {
+bool NullRunFunctionDeprecated(void* obj) {
webrtc::SleepMs(0); // Hand over timeslice, prevents busy looping.
return true;
}
+void NullRunFunction(void* obj) {}
+
// Function that sets a boolean.
-bool SetFlagRunFunction(void* obj) {
+bool SetFlagRunFunctionDeprecated(void* obj) {
bool* obj_as_bool = static_cast<bool*>(obj);
*obj_as_bool = true;
webrtc::SleepMs(0); // Hand over timeslice, prevents busy looping.
return true;
}
+
+void SetFlagRunFunction(void* obj) {
+ bool* obj_as_bool = static_cast<bool*>(obj);
+ *obj_as_bool = true;
+}
+
} // namespace
+TEST(PlatformThreadTest, StartStopDeprecated) {
+ PlatformThread thread(&NullRunFunctionDeprecated, nullptr,
+ "PlatformThreadTest");
+ EXPECT_TRUE(thread.name() == "PlatformThreadTest");
+ EXPECT_TRUE(thread.GetThreadRef() == 0);
+ thread.Start();
+ EXPECT_TRUE(thread.GetThreadRef() != 0);
+ thread.Stop();
+ EXPECT_TRUE(thread.GetThreadRef() == 0);
+}
+
+TEST(PlatformThreadTest, StartStop2Deprecated) {
+ PlatformThread thread1(&NullRunFunctionDeprecated, nullptr,
+ "PlatformThreadTest1");
+ PlatformThread thread2(&NullRunFunctionDeprecated, nullptr,
+ "PlatformThreadTest2");
+ EXPECT_TRUE(thread1.GetThreadRef() == thread2.GetThreadRef());
+ thread1.Start();
+ thread2.Start();
+ EXPECT_TRUE(thread1.GetThreadRef() != thread2.GetThreadRef());
+ thread2.Stop();
+ thread1.Stop();
+}
+
+TEST(PlatformThreadTest, RunFunctionIsCalledDeprecated) {
+ bool flag = false;
+ PlatformThread thread(&SetFlagRunFunctionDeprecated, &flag,
+ "RunFunctionIsCalled");
+ thread.Start();
+
+ // At this point, the flag may be either true or false.
+ thread.Stop();
+
+ // We expect the thread to have run at least once.
+ EXPECT_TRUE(flag);
+}
+
TEST(PlatformThreadTest, StartStop) {
PlatformThread thread(&NullRunFunction, nullptr, "PlatformThreadTest");
EXPECT_TRUE(thread.name() == "PlatformThreadTest");
@@ -62,4 +107,5 @@
// We expect the thread to have run at least once.
EXPECT_TRUE(flag);
}
+
} // rtc
diff --git a/webrtc/base/rate_limiter_unittest.cc b/webrtc/base/rate_limiter_unittest.cc
index 3c1b541..6d92567 100644
--- a/webrtc/base/rate_limiter_unittest.cc
+++ b/webrtc/base/rate_limiter_unittest.cc
@@ -130,9 +130,8 @@
rtc::Event end_signal_;
};
-bool RunTask(void* thread_task) {
+void RunTask(void* thread_task) {
reinterpret_cast<ThreadTask*>(thread_task)->Run();
- return false;
}
TEST_F(RateLimitTest, MultiThreadedUsage) {
diff --git a/webrtc/base/sequenced_task_checker_unittest.cc b/webrtc/base/sequenced_task_checker_unittest.cc
index 8588648..ae6e09d 100644
--- a/webrtc/base/sequenced_task_checker_unittest.cc
+++ b/webrtc/base/sequenced_task_checker_unittest.cc
@@ -35,14 +35,13 @@
}
private:
- static bool Run(void* obj) {
+ static void Run(void* obj) {
CallCalledSequentiallyOnThread* call_stuff_on_thread =
static_cast<CallCalledSequentiallyOnThread*>(obj);
EXPECT_EQ(
call_stuff_on_thread->expect_true_,
call_stuff_on_thread->sequenced_task_checker_->CalledSequentially());
call_stuff_on_thread->thread_has_run_event_.Set();
- return false;
}
const bool expect_true_;
diff --git a/webrtc/base/task_queue.h b/webrtc/base/task_queue.h
index a6a7036..5fcf00f 100644
--- a/webrtc/base/task_queue.h
+++ b/webrtc/base/task_queue.h
@@ -232,7 +232,7 @@
private:
#if defined(WEBRTC_BUILD_LIBEVENT)
- static bool ThreadMain(void* context);
+ static void ThreadMain(void* context);
static void OnWakeup(int socket, short flags, void* context); // NOLINT
static void RunTask(int fd, short flags, void* context); // NOLINT
static void RunTimer(int fd, short flags, void* context); // NOLINT
@@ -263,7 +263,7 @@
class MultimediaTimer;
typedef std::unordered_map<UINT_PTR, std::unique_ptr<QueuedTask>>
DelayedTasks;
- static bool ThreadMain(void* context);
+ static void ThreadMain(void* context);
static bool ProcessQueuedMessages(DelayedTasks* delayed_tasks,
std::vector<MultimediaTimer>* timers);
diff --git a/webrtc/base/task_queue_libevent.cc b/webrtc/base/task_queue_libevent.cc
index f3ab89b..c802588 100644
--- a/webrtc/base/task_queue_libevent.cc
+++ b/webrtc/base/task_queue_libevent.cc
@@ -256,7 +256,7 @@
}
// static
-bool TaskQueue::ThreadMain(void* context) {
+void TaskQueue::ThreadMain(void* context) {
TaskQueue* me = static_cast<TaskQueue*>(context);
QueueContext queue_context(me);
@@ -269,8 +269,6 @@
for (TimerEvent* timer : queue_context.pending_timers_)
delete timer;
-
- return false;
}
// static
diff --git a/webrtc/base/task_queue_win.cc b/webrtc/base/task_queue_win.cc
index 11aa81d..c8ef721 100644
--- a/webrtc/base/task_queue_win.cc
+++ b/webrtc/base/task_queue_win.cc
@@ -228,7 +228,7 @@
}
// static
-bool TaskQueue::ThreadMain(void* context) {
+void TaskQueue::ThreadMain(void* context) {
HANDLE timer_handles[MultimediaTimer::kMaxTimers];
// Active multimedia timers.
std::vector<MultimediaTimer> mm_timers;
@@ -283,8 +283,6 @@
RTC_DCHECK_EQ(WAIT_IO_COMPLETION, result);
}
}
-
- return false;
}
// static