Use Begin/End semantics for synthetic delays

base/debug/trace_event_synthetic_delay.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/debug/trace_event_synthetic_delay.h"
 #include "base/memory/singleton.h"
-#include "base/threading/platform_thread.h"
-#include "base/threading/thread_local.h"
 
 namespace {
 const int kMaxSyntheticDelays = 32;
 }  // namespace
 
 namespace base {
 namespace debug {
 
 TraceEventSyntheticDelayClock::TraceEventSyntheticDelayClock() {}
 TraceEventSyntheticDelayClock::~TraceEventSyntheticDelayClock() {}
 
-// Thread-local state for each synthetic delay point. This allows the same delay
-// to be applied to multiple threads simultaneously.
-struct ThreadState {

[brianderson, 2013/12/19 00:16:59]

Since you are removing the thread-local state, does that mean delays of the same
category share state (end times) across threads? Do we not need to worry about
categories being applied independently on separate threads anymore?

[Sami, 2013/12/19 11:14:59]

Yes, now the only way to apply a specific delay independently on several threads
is to use the parallel versions Begin/End. Turns out with the delay points we
have now, there isn't really a need to run them in parallel except in the raster
worker pool. Note that we still need thread safety, e.g., for async texture
uploads where Begin and End are called on different threads.

Now that I think about it, there's a way to make all this a little less
surprising by making the scoped delay (TRACE_EVENT_SYNTHETIC_DELAY) use the
parallel path. This way you can sprinkle the scoped delays pretty much anywhere
and only need to worry about threading for the other variants.

-  ThreadState();
-
-  base::TimeTicks start_time;
-  unsigned trigger_count;
-  int generation;
-};
-
-ThreadState::ThreadState() : trigger_count(0u), generation(0) {}
-
 class TraceEventSyntheticDelayRegistry : public TraceEventSyntheticDelayClock {
  public:
   static TraceEventSyntheticDelayRegistry* GetInstance();
 
   TraceEventSyntheticDelay* GetOrCreateDelay(const char* name);
-  ThreadState* GetThreadState(int index);
+  void ResetAllDelays();
 
   // TraceEventSyntheticDelayClock implementation.
   virtual base::TimeTicks Now() OVERRIDE;
 
  private:
   TraceEventSyntheticDelayRegistry();
 
   friend struct DefaultSingletonTraits<TraceEventSyntheticDelayRegistry>;
 
   Lock lock_;
   TraceEventSyntheticDelay delays_[kMaxSyntheticDelays];
   TraceEventSyntheticDelay dummy_delay_;
   base::subtle::Atomic32 delay_count_;
 
-  ThreadLocalPointer<ThreadState> thread_states_;
-
   DISALLOW_COPY_AND_ASSIGN(TraceEventSyntheticDelayRegistry);
 };
 
 TraceEventSyntheticDelay::TraceEventSyntheticDelay()
-    : mode_(STATIC), generation_(0), thread_state_index_(0), clock_(NULL) {}
+    : mode_(STATIC), begin_count_(0), trigger_count_(0), clock_(NULL) {}
 
 TraceEventSyntheticDelay::~TraceEventSyntheticDelay() {}
 
 TraceEventSyntheticDelay* TraceEventSyntheticDelay::Lookup(
     const std::string& name) {
   return TraceEventSyntheticDelayRegistry::GetInstance()->GetOrCreateDelay(
       name.c_str());
 }
 
 void TraceEventSyntheticDelay::Initialize(
     const std::string& name,
-    TraceEventSyntheticDelayClock* clock,
-    int thread_state_index) {
+    TraceEventSyntheticDelayClock* clock) {
   name_ = name;
   clock_ = clock;
-  thread_state_index_ = thread_state_index;
 }
 
 void TraceEventSyntheticDelay::SetTargetDuration(
     base::TimeDelta target_duration) {
   AutoLock lock(lock_);
   target_duration_ = target_duration;
-  generation_++;
+  trigger_count_ = 0;
+  begin_count_ = 0;
 }
 
 void TraceEventSyntheticDelay::SetMode(Mode mode) {
   AutoLock lock(lock_);
   mode_ = mode;
-  generation_++;
 }
 
 void TraceEventSyntheticDelay::SetClock(TraceEventSyntheticDelayClock* clock) {
   AutoLock lock(lock_);
   clock_ = clock;
-  generation_++;
 }
 
-void TraceEventSyntheticDelay::Activate() {
+void TraceEventSyntheticDelay::Begin() {
   // Note that we check for a non-zero target duration without locking to keep
   // things quick for the common case when delays are disabled. Since the delay
   // calculation is done with a lock held, it will always be correct. The only
   // downside of this is that we may fail to apply some delays when the target
   // duration changes.
   ANNOTATE_BENIGN_RACE(&target_duration_, "Synthetic delay duration");
   if (!target_duration_.ToInternalValue())
     return;
 
-  ThreadState* thread_state =
-      TraceEventSyntheticDelayRegistry::GetInstance()->
-          GetThreadState(thread_state_index_);
-  if (!thread_state->start_time.ToInternalValue())
-    thread_state->start_time = clock_->Now();
+  base::TimeTicks start_time = clock_->Now();
+  {
+    AutoLock lock(lock_);
+    if (++begin_count_ != 1)
+      return;
+    end_time_ = CalculateEndTimeLocked(start_time);
+  }
 }
 
-void TraceEventSyntheticDelay::Apply() {
+void TraceEventSyntheticDelay::BeginParallel(base::TimeTicks* out_end_time) {
+  // See note in Begin().
+  ANNOTATE_BENIGN_RACE(&target_duration_, "Synthetic delay duration");
+  if (!target_duration_.ToInternalValue()) {
+    *out_end_time = base::TimeTicks();
+    return;
+  }
+
+  base::TimeTicks start_time = clock_->Now();
+  {
+    AutoLock lock(lock_);
+    *out_end_time = CalculateEndTimeLocked(start_time);
+  }
+}
+
+void TraceEventSyntheticDelay::End() {
+  // See note in Begin().
   ANNOTATE_BENIGN_RACE(&target_duration_, "Synthetic delay duration");
   if (!target_duration_.ToInternalValue())
     return;
 
-  ThreadState* thread_state =
-      TraceEventSyntheticDelayRegistry::GetInstance()->
-          GetThreadState(thread_state_index_);
-  if (!thread_state->start_time.ToInternalValue())
-    return;
-  base::TimeTicks now = clock_->Now();
-  base::TimeTicks start_time = thread_state->start_time;
   base::TimeTicks end_time;
-  thread_state->start_time = base::TimeTicks();
-
   {
     AutoLock lock(lock_);
-    if (thread_state->generation != generation_) {
-      thread_state->trigger_count = 0;
-      thread_state->generation = generation_;
-    }
+    if (!begin_count_ || --begin_count_ != 0)
+      return;
+    end_time = end_time_;
+  }
+  if (!end_time.is_null())
+    ApplyDelay(end_time);
+}
 
-    if (mode_ == ONE_SHOT && thread_state->trigger_count++)
-      return;
-    else if (mode_ == ALTERNATING && thread_state->trigger_count++ % 2)
-      return;
+void TraceEventSyntheticDelay::EndParallel(base::TimeTicks end_time) {
+  if (!end_time.is_null())
+    ApplyDelay(end_time);

[brianderson, 2013/12/19 00:16:59]

I like how you make the client worry about about the cross thread handling. It's
much cleaner and more explicit.

[Sami, 2013/12/19 11:14:59]

Thanks. It took a few tries to get here but I think the model looks good now :)

+}
 
-    end_time = start_time + target_duration_;
-    if (now >= end_time)
-      return;
-  }
-  ApplyDelay(end_time);
+base::TimeTicks TraceEventSyntheticDelay::CalculateEndTimeLocked(
+    base::TimeTicks start_time) {
+  if (mode_ == ONE_SHOT && trigger_count_++)
+    return base::TimeTicks();
+  else if (mode_ == ALTERNATING && trigger_count_++ % 2)
+    return base::TimeTicks();
+  return start_time + target_duration_;
 }
 
 void TraceEventSyntheticDelay::ApplyDelay(base::TimeTicks end_time) {
   TRACE_EVENT0("synthetic_delay", name_.c_str());
   while (clock_->Now() < end_time) {
     // Busy loop.
   }
 }
 
 TraceEventSyntheticDelayRegistry*
@@ skipping 21 matching lines @@
   for (int i = 0; i < delay_count; ++i) {
     if (!strcmp(name, delays_[i].name_.c_str()))
       return &delays_[i];
   }
 
   DCHECK(delay_count < kMaxSyntheticDelays)
       << "must increase kMaxSyntheticDelays";
   if (delay_count >= kMaxSyntheticDelays)
     return &dummy_delay_;
 
-  delays_[delay_count].Initialize(std::string(name), this, delay_count);
+  delays_[delay_count].Initialize(std::string(name), this);
   base::subtle::Release_Store(&delay_count_, delay_count + 1);
   return &delays_[delay_count];
 }
 
-ThreadState* TraceEventSyntheticDelayRegistry::GetThreadState(int index) {
-  DCHECK(index >= 0 && index < kMaxSyntheticDelays);
-  if (index < 0 || index >= kMaxSyntheticDelays)
-    return NULL;
-
-  // Note that these thread states are leaked at program exit. They will only
-  // get allocated if synthetic delays are actually used.
-  ThreadState* thread_states = thread_states_.Get();
-  if (!thread_states)
-    thread_states_.Set((thread_states = new ThreadState[kMaxSyntheticDelays]));
-  return &thread_states[index];
-}
-
 base::TimeTicks TraceEventSyntheticDelayRegistry::Now() {
   return base::TimeTicks::HighResNow();
 }
 
+void TraceEventSyntheticDelayRegistry::ResetAllDelays() {
+  AutoLock lock(lock_);
+  int delay_count = base::subtle::Acquire_Load(&delay_count_);
+  for (int i = 0; i < delay_count; ++i)
+    delays_[i].SetTargetDuration(base::TimeDelta());
+}
+
+void ResetTraceEventSyntheticDelays() {
+  TraceEventSyntheticDelayRegistry::GetInstance()->ResetAllDelays();
+}
+
 }  // namespace debug
 }  // namespace base
 
 namespace trace_event_internal {
 
 ScopedSyntheticDelay::ScopedSyntheticDelay(const char* name,
                                            base::subtle::AtomicWord* impl_ptr)
     : delay_impl_(GetOrCreateDelay(name, impl_ptr)) {
-  delay_impl_->Activate();
+  delay_impl_->Begin();
 }
 
-ScopedSyntheticDelay::~ScopedSyntheticDelay() { delay_impl_->Apply(); }
+ScopedSyntheticDelay::~ScopedSyntheticDelay() { delay_impl_->End(); }
 
 base::debug::TraceEventSyntheticDelay* GetOrCreateDelay(
     const char* name,
     base::subtle::AtomicWord* impl_ptr) {
   base::debug::TraceEventSyntheticDelay* delay_impl =
       reinterpret_cast<base::debug::TraceEventSyntheticDelay*>(
           base::subtle::NoBarrier_Load(impl_ptr));
   if (!delay_impl) {
     delay_impl = base::debug::TraceEventSyntheticDelayRegistry::GetInstance()
                      ->GetOrCreateDelay(name);
     base::subtle::NoBarrier_Store(
         impl_ptr, reinterpret_cast<base::subtle::AtomicWord>(delay_impl));
   }
   return delay_impl;
 }
 
 }  // namespace trace_event_internal