Added disjoint context class which manages disjoints within gpu timing.

gpu/command_buffer/service/gpu_timing.cc

 // Copyright (c) 2015 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 "gpu/command_buffer/service/gpu_timing.h"
 
 #include "base/time/time.h"
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_version_info.h"
@@ skipping 45 matching lines @@
   *end = (end_stamp / base::Time::kNanosecondsPerMicrosecond) + offset_;
 }
 
 int64 GPUTimer::GetDeltaElapsed() {
   int64 start = 0;
   int64 end = 0;
   GetStartEndTimestamps(&start, &end);
   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();
 
-  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;
-    return true;
+  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;
+    }
   }
-  return false;
 }
 
 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;
+}
+
+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 {
   switch (timer_type_) {
     case kTimerTypeDisjoint:
       return "GL_EXT_disjoint_timer_query";
     case kTimerTypeARB:
       return "GL_ARB_timer_query";
     default:
       return "Unknown";
   }
 }
 
-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;
     glGetInteger64v(GL_TIMESTAMP, &gl_now);
     int64 now =
         cpu_time_for_testing_.is_null()
             ? base::TimeTicks::NowFromSystemTraceTime().ToInternalValue()
             : cpu_time_for_testing_.Run();
     offset_ = now - gl_now / base::Time::kNanosecondsPerMicrosecond;
     offset_valid_ = timer_type_ == kTimerTypeARB;
   }
   return offset_;
 }
 
 void GPUTiming::InvalidateTimerOffset() {
   offset_valid_ = false;
 }
 
 void GPUTiming::SetCpuTimeForTesting(
     const base::Callback<int64(void)>& cpu_time) {
   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