From 25819b829494f4563d99807f1f035e30b13bb4f3 Mon Sep 17 00:00:00 2001 From: "tommi@webrtc.org" Date: Tue, 17 Mar 2015 15:35:11 +0000 Subject: [PATCH] Revert 8753 "Use atomic operations for setting/reading the trace..." Caused VP9 test to fail on TSAN and doesn't build in some configuration due to "../webrtc/base/criticalsection.h:181:12: error: cannot compile this atomic library call yet" :-( > Use atomic operations for setting/reading the trace filter. > The filter is currently being set and read by a number of threads and tripping up tsan. > > R=mflodman@webrtc.org > BUG= > > Review URL: https://webrtc-codereview.appspot.com/47609004 TBR=tommi@webrtc.org Review URL: https://webrtc-codereview.appspot.com/51369004 Cr-Commit-Position: refs/heads/master@{#8759} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8759 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/base/criticalsection.h | 18 +++--------------- webrtc/system_wrappers/interface/trace.h | 6 +++--- webrtc/system_wrappers/source/trace_impl.cc | 16 +--------------- 3 files changed, 7 insertions(+), 33 deletions(-) diff --git a/webrtc/base/criticalsection.h b/webrtc/base/criticalsection.h index 441845385..088e72e14 100644 --- a/webrtc/base/criticalsection.h +++ b/webrtc/base/criticalsection.h @@ -163,31 +163,19 @@ class AtomicOps { static int Decrement(volatile int* i) { return ::InterlockedDecrement(reinterpret_cast(i)); } - - // No barrier load. static int Load(volatile const int* i) { return *i; } - - // No barrier store. - static void Store(volatile int* i, int value) { - *i = value; - } #else static int Increment(volatile int* i) { - return __atomic_add_fetch(i, 1, __ATOMIC_SEQ_CST); + return __sync_add_and_fetch(i, 1); } static int Decrement(volatile int* i) { - return __atomic_sub_fetch(i, 1, __ATOMIC_SEQ_CST); + return __sync_sub_and_fetch(i, 1); } - // No barrier load. static int Load(volatile const int* i) { // Adding 0 is a no-op, so const_cast is fine. - return __atomic_load_n(const_cast(i), 0); - } - // No barrier store. - static void Store(volatile int* i, int value) { - __atomic_store_n(i, value, __ATOMIC_RELAXED); + return __sync_add_and_fetch(const_cast(i), 0); } #endif }; diff --git a/webrtc/system_wrappers/interface/trace.h b/webrtc/system_wrappers/interface/trace.h index e63b603d1..44ea658bd 100644 --- a/webrtc/system_wrappers/interface/trace.h +++ b/webrtc/system_wrappers/interface/trace.h @@ -49,10 +49,10 @@ class Trace { // filter parameter is a bitmask where each message type is enumerated by the // TraceLevel enumerator. TODO(hellner): why is the TraceLevel enumerator not // defined in this file? - static void set_level_filter(int filter); + static void set_level_filter(uint32_t filter) { level_filter_ = filter; } // Returns what type of messages are written to the trace file. - static int level_filter(); + static uint32_t level_filter() { return level_filter_; } // Sets the file name. If add_file_counter is false the same file will be // reused when it fills up. If it's true a new file with incremented name @@ -84,7 +84,7 @@ class Trace { const char* msg, ...); private: - static volatile int level_filter_; + static uint32_t level_filter_; }; } // namespace webrtc diff --git a/webrtc/system_wrappers/source/trace_impl.cc b/webrtc/system_wrappers/source/trace_impl.cc index 6a293ccbd..c41159dae 100644 --- a/webrtc/system_wrappers/source/trace_impl.cc +++ b/webrtc/system_wrappers/source/trace_impl.cc @@ -32,7 +32,7 @@ namespace webrtc { const int Trace::kBoilerplateLength = 71; const int Trace::kTimestampPosition = 13; const int Trace::kTimestampLength = 12; -volatile int Trace::level_filter_ = kTraceDefault; +uint32_t Trace::level_filter_ = kTraceDefault; // Construct On First Use idiom. Avoids "static initialization order fiasco". TraceImpl* TraceImpl::StaticInstance(CountOperation count_operation, @@ -518,17 +518,14 @@ bool TraceImpl::CreateFileName( return true; } -// static void Trace::CreateTrace() { TraceImpl::StaticInstance(kAddRef); } -// static void Trace::ReturnTrace() { TraceImpl::StaticInstance(kRelease); } -// static int32_t Trace::TraceFile(char file_name[FileWrapper::kMaxFileNameSize]) { TraceImpl* trace = TraceImpl::GetTrace(); if (trace) { @@ -539,17 +536,6 @@ int32_t Trace::TraceFile(char file_name[FileWrapper::kMaxFileNameSize]) { return -1; } -// static -void Trace::set_level_filter(int filter) { - rtc::AtomicOps::Store(&level_filter_, filter); -} - -// static -int Trace::level_filter() { - return rtc::AtomicOps::Load(&level_filter_); -} - -// static int32_t Trace::SetTraceFile(const char* file_name, const bool add_file_counter) { TraceImpl* trace = TraceImpl::GetTrace();