Added disjoint context class which manages disjoints within gpu timing.

gpu/command_buffer/service/gpu_timing.cc

Index: gpu/command_buffer/service/gpu_timing.cc
diff --git a/gpu/command_buffer/service/gpu_timing.cc b/gpu/command_buffer/service/gpu_timing.cc
index 6ab3e831f387515c351e1319dd584725f2533d1b..b330c45a78719e46ba9eb576680ef3681bf49d42 100644
--- a/gpu/command_buffer/service/gpu_timing.cc
+++ b/gpu/command_buffer/service/gpu_timing.cc
@@ -63,31 +63,94 @@ int64 GPUTimer::GetDeltaElapsed() {
   return end - start;
 }
 
-GPUTiming::GPUTiming() : cpu_time_for_testing_() {
+GPUTiming::GPUTiming(gfx::GLContext* gl_context) {
+  Initialize(gl_context);

[Daniele Castagna, 2015/02/23 22:21:19]

Why do you initialize the object here?
gl_context defaults to nullptr, so when you create the object with the default
parameter you're basically just initializing the members of this object twice.

I'd go with one of those two options:
- Initialize is public and takes a context. The constructor doesn't take a gl
context.
- Initialize is private, and the object gets initialized when is create from the
constructor that takes a context.

 }
 
 GPUTiming::~GPUTiming() {
+  DCHECK(GetDisjointContextIDCount() == 0);
 }
 
-bool GPUTiming::Initialize(gfx::GLContext* gl_context) {
-  DCHECK(gl_context);
-  DCHECK_EQ(kTimerTypeInvalid, timer_type_);
+void GPUTiming::Initialize(gfx::GLContext* gl_context) {

[Daniele Castagna, 2015/02/23 22:21:19]

Who's suppose to call this function? You call it from the constructor, with
gl_context set to null, and then from gpu_tracer code in intialize with a real
gl_context.

+  timer_type_ = kTimerTypeInvalid;
+  offset_ = 0;  // offset cache when timer_type_ == kTimerTypeARB
+  offset_valid_ = false;
+  disjoint_occurred_ = false;
+  disjoint_contexts_.clear();
+  cpu_time_for_testing_.Reset();
+
+  if (gl_context) {
+    const gfx::GLVersionInfo* version_info = gl_context->GetVersionInfo();
+    DCHECK(version_info);
+
+    timer_type_ = kTimerTypeInvalid;
+    if (version_info->is_es3 &&  // glGetInteger64v is supported under ES3.
+        gfx::g_driver_gl.ext.b_GL_EXT_disjoint_timer_query) {
+      timer_type_ = kTimerTypeDisjoint;
+    } else if (gfx::g_driver_gl.ext.b_GL_ARB_timer_query) {
+      timer_type_ = kTimerTypeARB;
+    }
+  }
+}
 
-  const gfx::GLVersionInfo* version_info = gl_context->GetVersionInfo();
-  DCHECK(version_info);
-  if (version_info->is_es3 &&  // glGetInteger64v is supported under ES3.
-      gfx::g_driver_gl.ext.b_GL_EXT_disjoint_timer_query) {
-    timer_type_ = kTimerTypeDisjoint;
-    return true;
-  } else if (gfx::g_driver_gl.ext.b_GL_ARB_timer_query) {
-    timer_type_ = kTimerTypeARB;
+bool GPUTiming::IsAvailable() {
+  return timer_type_ != kTimerTypeInvalid;
+}
+
+int GPUTiming::CreateDisjointContextID() {
+  const int num_contexts = static_cast<int>(disjoint_contexts_.size());
+  for (int i = 0; i < num_contexts; ++i) {

[Daniele Castagna, 2015/02/23 22:21:19]

What about using std::find_if?

+    DisjointContext& context = disjoint_contexts_[i];
+    if (!context.valid_) {
+      context.valid_ = true;
+      context.disjoint_value_ = disjoint_occurred_;
+      return i;
+    }
+  }
+  disjoint_contexts_.push_back(DisjointContext(disjoint_occurred_));
+  return num_contexts;
+}
+
+bool GPUTiming::ReleaseDisjointContextID(int disjoint_context_id) {

[Daniele Castagna, 2015/02/23 22:21:19]

Could you test this in your test?

+  if (disjoint_context_id >= 0 &&
+      disjoint_context_id < static_cast<int>(disjoint_contexts_.size()) &&
+      disjoint_contexts_[disjoint_context_id].valid_) {
+    disjoint_contexts_[disjoint_context_id].valid_ = false;
     return true;
   }
   return false;
 }
 
-bool GPUTiming::IsAvailable() {
-  return timer_type_ != kTimerTypeInvalid;
+int GPUTiming::GetDisjointContextIDCount() const {
+  int count = 0;

[Daniele Castagna, 2015/02/23 22:21:19]

std::count_if?

+  for (const DisjointContext& context : disjoint_contexts_) {
+    if (context.disjoint_value_) {
+      ++count;
+    }
+  }
+  return count;
+}
+
+bool GPUTiming::CheckAndResetTimerErrors(int disjoint_context_id) {
+  if (timer_type_ == kTimerTypeDisjoint &&

[Daniele Castagna, 2015/02/23 22:21:19]

If the timer_type_ is disjoint, and the context_id is invalid this method is
going to return false. We should probably make sure that the context_id is
always valid.

+      disjoint_context_id >= 0 &&
+      disjoint_context_id < static_cast<int>(disjoint_contexts_.size()) &&
+      disjoint_contexts_[disjoint_context_id].valid_) {
+    GLint disjoint_value = 0;
+    glGetIntegerv(GL_GPU_DISJOINT_EXT, &disjoint_value);
+    if (disjoint_value != 0) {
+      disjoint_occurred_ = true;
+      for (DisjointContext& context : disjoint_contexts_) {
+        context.disjoint_value_ = true;
+      }
+    }
+
+    DisjointContext& context = disjoint_contexts_[disjoint_context_id];
+    const bool prev_disjoint_value = context.disjoint_value_;
+    context.disjoint_value_ = false;
+    return prev_disjoint_value;
+  }
+  return false;
 }
 
 const char* GPUTiming::GetTimerTypeName() const {
@@ -101,16 +164,6 @@ const char* GPUTiming::GetTimerTypeName() const {
   }
 }
 
-bool GPUTiming::CheckAndResetTimerErrors() {
-  if (timer_type_ == kTimerTypeDisjoint) {
-    GLint disjoint_value = 0;
-    glGetIntegerv(GL_GPU_DISJOINT_EXT, &disjoint_value);
-    return disjoint_value != 0;
-  } else {
-    return false;
-  }
-}
-
 int64 GPUTiming::CalculateTimerOffset() {
   if (!offset_valid_) {
     GLint64 gl_now = 0;
@@ -134,13 +187,4 @@ void GPUTiming::SetCpuTimeForTesting(
   cpu_time_for_testing_ = cpu_time;
 }
 
-void GPUTiming::SetOffsetForTesting(int64 offset, bool cache_it) {
-  offset_ = offset;
-  offset_valid_ = cache_it;
-}
-
-void GPUTiming::SetTimerTypeForTesting(TimerType type) {
-  timer_type_ = type;
-}
-
 }  // namespace gpu