From e791ffd63873739b47a2633531f4ac2f12e6951d Mon Sep 17 00:00:00 2001 From: sprang Date: Tue, 26 Jan 2016 01:53:20 -0800 Subject: [PATCH] Remove non-monotonic clock support Real time clock may cause problems as they can move (even backwards) if the clock is changed, eg updated by NTP. Non-monotonic clocks still in use on some platform (I'm looking at you, Apple) for timed waits, but that should be less of an issue than actual timestamps. BUG=webrtc:5452 Review URL: https://codereview.webrtc.org/1613013002 Cr-Commit-Position: refs/heads/master@{#11375} --- webrtc/base/platform_thread.cc | 2 +- webrtc/system_wrappers/BUILD.gn | 22 ++----------- .../source/event_timer_posix.cc | 33 +++++++++++-------- webrtc/system_wrappers/source/tick_util.cc | 5 --- webrtc/system_wrappers/system_wrappers.gyp | 9 ----- 5 files changed, 23 insertions(+), 48 deletions(-) diff --git a/webrtc/base/platform_thread.cc b/webrtc/base/platform_thread.cc index 05b7a258c0..b6fd8732aa 100644 --- a/webrtc/base/platform_thread.cc +++ b/webrtc/base/platform_thread.cc @@ -179,7 +179,7 @@ void PlatformThread::Run() { if (!name_.empty()) rtc::SetCurrentThreadName(name_.c_str()); do { - // The interface contract of Start/Stop is that for a successfull call to + // 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_)) diff --git a/webrtc/system_wrappers/BUILD.gn b/webrtc/system_wrappers/BUILD.gn index ef03269f1e..b288230820 100644 --- a/webrtc/system_wrappers/BUILD.gn +++ b/webrtc/system_wrappers/BUILD.gn @@ -99,15 +99,7 @@ static_library("system_wrappers") { "source/logcat_trace_context.cc", ] - defines += [ - "WEBRTC_THREAD_RR", - - # TODO(leozwang): Investigate CLOCK_REALTIME and CLOCK_MONOTONIC - # support on Android. Keep WEBRTC_CLOCK_TYPE_REALTIME for now, - # remove it after I verify that CLOCK_MONOTONIC is fully functional - # with condition and event functions in system_wrappers. - "WEBRTC_CLOCK_TYPE_REALTIME", - ] + defines += [ "WEBRTC_THREAD_RR" ] deps += [ ":cpu_features_android" ] @@ -115,12 +107,7 @@ static_library("system_wrappers") { } if (is_linux) { - defines += [ - "WEBRTC_THREAD_RR", - # TODO(andrew): can we select this automatically? - # Define this if the Linux system does not support CLOCK_MONOTONIC. - #"WEBRTC_CLOCK_TYPE_REALTIME", - ] + defines += [ "WEBRTC_THREAD_RR" ] libs += [ "rt" ] } @@ -130,10 +117,7 @@ static_library("system_wrappers") { } if (is_ios || is_mac) { - defines += [ - "WEBRTC_THREAD_RR", - "WEBRTC_CLOCK_TYPE_REALTIME", - ] + defines += [ "WEBRTC_THREAD_RR" ] } if (is_ios) { diff --git a/webrtc/system_wrappers/source/event_timer_posix.cc b/webrtc/system_wrappers/source/event_timer_posix.cc index 9f9a324bcb..990ff725fb 100644 --- a/webrtc/system_wrappers/source/event_timer_posix.cc +++ b/webrtc/system_wrappers/source/event_timer_posix.cc @@ -41,15 +41,18 @@ EventTimerPosix::EventTimerPosix() pthread_mutexattr_init(&attr); pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); pthread_mutex_init(&mutex_, &attr); -#ifdef WEBRTC_CLOCK_TYPE_REALTIME - pthread_cond_init(&cond_, 0); -#else pthread_condattr_t cond_attr; pthread_condattr_init(&cond_attr); +// TODO(sprang): Remove HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC special case once +// all supported Android platforms support pthread_condattr_setclock. +// TODO(sprang): Add support for monotonic clock on Apple platforms. +#if !(defined(WEBRTC_MAC) || defined(WEBRTC_IOS)) && \ + !(defined(WEBRTC_ANDROID) && \ + defined(HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC)) pthread_condattr_setclock(&cond_attr, CLOCK_MONOTONIC); +#endif pthread_cond_init(&cond_, &cond_attr); pthread_condattr_destroy(&cond_attr); -#endif } EventTimerPosix::~EventTimerPosix() { @@ -75,11 +78,7 @@ EventTypeWrapper EventTimerPosix::Wait(unsigned long timeout) { if (WEBRTC_EVENT_INFINITE != timeout) { timespec end_at; #ifndef WEBRTC_MAC -#ifdef WEBRTC_CLOCK_TYPE_REALTIME - clock_gettime(CLOCK_REALTIME, &end_at); -#else clock_gettime(CLOCK_MONOTONIC, &end_at); -#endif #else timeval value; struct timezone time_zone; @@ -95,8 +94,13 @@ EventTypeWrapper EventTimerPosix::Wait(unsigned long timeout) { end_at.tv_sec++; end_at.tv_nsec -= E9; } - while (ret_val == 0 && !event_set_) + while (ret_val == 0 && !event_set_) { +#if defined(WEBRTC_ANDROID) && defined(HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC) + ret_val = pthread_cond_timedwait_monotonic_np(&cond_, &mutex_, &end_at); +#else ret_val = pthread_cond_timedwait(&cond_, &mutex_, &end_at); +#endif // WEBRTC_ANDROID && HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC + } } else { while (ret_val == 0 && !event_set_) ret_val = pthread_cond_wait(&cond_, &mutex_); @@ -119,8 +123,13 @@ EventTypeWrapper EventTimerPosix::Wait(timespec* end_at) { int ret_val = 0; RTC_CHECK_EQ(0, pthread_mutex_lock(&mutex_)); - while (ret_val == 0 && !event_set_) + while (ret_val == 0 && !event_set_) { +#if defined(WEBRTC_ANDROID) && defined(HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC) + ret_val = pthread_cond_timedwait_monotonic_np(&cond_, &mutex_, end_at); +#else ret_val = pthread_cond_timedwait(&cond_, &mutex_, end_at); +#endif // WEBRTC_ANDROID && HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC + } RTC_DCHECK(ret_val == 0 || ret_val == ETIMEDOUT); @@ -172,11 +181,7 @@ bool EventTimerPosix::Process() { pthread_mutex_lock(&mutex_); if (created_at_.tv_sec == 0) { #ifndef WEBRTC_MAC -#ifdef WEBRTC_CLOCK_TYPE_REALTIME - clock_gettime(CLOCK_REALTIME, &created_at_); -#else clock_gettime(CLOCK_MONOTONIC, &created_at_); -#endif #else timeval value; struct timezone time_zone; diff --git a/webrtc/system_wrappers/source/tick_util.cc b/webrtc/system_wrappers/source/tick_util.cc index 8d94289417..c02b5314a4 100644 --- a/webrtc/system_wrappers/source/tick_util.cc +++ b/webrtc/system_wrappers/source/tick_util.cc @@ -106,12 +106,7 @@ int64_t TickTime::QueryOsForTicks() { return now + (num_wrap_time_get_time << 32); #elif defined(WEBRTC_LINUX) struct timespec ts; - // TODO(wu): Remove CLOCK_REALTIME implementation. -#ifdef WEBRTC_CLOCK_TYPE_REALTIME - clock_gettime(CLOCK_REALTIME, &ts); -#else clock_gettime(CLOCK_MONOTONIC, &ts); -#endif return 1000000000LL * ts.tv_sec + ts.tv_nsec; #elif defined(WEBRTC_MAC) // Return absolute time as an offset from the first call to this function, so diff --git a/webrtc/system_wrappers/system_wrappers.gyp b/webrtc/system_wrappers/system_wrappers.gyp index f530d6d6a4..dc0de49957 100644 --- a/webrtc/system_wrappers/system_wrappers.gyp +++ b/webrtc/system_wrappers/system_wrappers.gyp @@ -97,11 +97,6 @@ ['OS=="android"', { 'defines': [ 'WEBRTC_THREAD_RR', - # TODO(leozwang): Investigate CLOCK_REALTIME and CLOCK_MONOTONIC - # support on Android. Keep WEBRTC_CLOCK_TYPE_REALTIME for now, - # remove it after I verify that CLOCK_MONOTONIC is fully functional - # with condition and event functions in system_wrappers. - 'WEBRTC_CLOCK_TYPE_REALTIME', ], 'conditions': [ ['build_with_chromium==1', { @@ -128,9 +123,6 @@ ['OS=="linux"', { 'defines': [ 'WEBRTC_THREAD_RR', - # TODO(andrew): can we select this automatically? - # Define this if the Linux system does not support CLOCK_MONOTONIC. - #'WEBRTC_CLOCK_TYPE_REALTIME', ], 'link_settings': { 'libraries': [ '-lrt', ], @@ -147,7 +139,6 @@ ['OS=="ios" or OS=="mac"', { 'defines': [ 'WEBRTC_THREAD_RR', - 'WEBRTC_CLOCK_TYPE_REALTIME', ], }], ['OS=="win"', {