Simplified GPU Tracer by removing parent base class.

gpu/command_buffer/service/gpu_tracer.cc

Index: gpu/command_buffer/service/gpu_tracer.cc
diff --git a/gpu/command_buffer/service/gpu_tracer.cc b/gpu/command_buffer/service/gpu_tracer.cc
index 2cd38e79ca470b6a5c595541c5c3a5c8f640d320..7bbd83e416782b0bcd23c503ccd1fcbc73fb3ace 100644
--- a/gpu/command_buffer/service/gpu_tracer.cc
+++ b/gpu/command_buffer/service/gpu_tracer.cc
@@ -74,146 +74,41 @@ class NoopTrace : public Trace {
   DISALLOW_COPY_AND_ASSIGN(NoopTrace);
 };
 
-struct TraceMarker {
-  TraceMarker(const std::string& name, GpuTracerSource source)
-      : name_(name), source_(source) {}
-
-  std::string name_;
-  GpuTracerSource source_;
-  scoped_refptr<Trace> trace_;
-};
-
-class GPUTracerImpl
-    : public GPUTracer,
-      public base::SupportsWeakPtr<GPUTracerImpl> {
- public:
-  GPUTracerImpl()
-      : gpu_trace_srv_category(TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
-            TRACE_DISABLED_BY_DEFAULT("gpu.service"))),
-        gpu_trace_dev_category(TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
-            TRACE_DISABLED_BY_DEFAULT("gpu.device"))),
-        gpu_executing_(false),
-        process_posted_(false) {}
-  virtual ~GPUTracerImpl() {}
-
-  // Implementation of gpu::gles2::GPUTracer
-  virtual bool BeginDecoding() OVERRIDE;
-  virtual bool EndDecoding() OVERRIDE;
-  virtual bool Begin(const std::string& name, GpuTracerSource source) OVERRIDE;
-  virtual bool End(GpuTracerSource source) OVERRIDE;
-  virtual const std::string& CurrentName() const OVERRIDE;
-  virtual bool IsTracing() OVERRIDE {
-    return (*gpu_trace_srv_category != 0) || (*gpu_trace_dev_category != 0);
+GPUTracer::GPUTracer(gles2::GLES2Decoder* decoder)
+    : gpu_trace_srv_category(TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
+          TRACE_DISABLED_BY_DEFAULT("gpu.service"))),
+      gpu_trace_dev_category(TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
+          TRACE_DISABLED_BY_DEFAULT("gpu.device"))),
+      decoder_(decoder),
+      timer_offset_(0),
+      last_tracer_source_(kTraceGroupInvalid),
+      enabled_(false),
+      gpu_timing_synced_(false),
+      gpu_executing_(false),
+      process_posted_(false) {
+  if (gfx::g_driver_gl.ext.b_GL_ARB_timer_query) {
+    outputter_ = TraceOutputter::Create("GL_ARB_timer_query");
+    enabled_ = true;
   }
-  virtual void CalculateTimerOffset() {}
-
-  // Process any completed traces.
-  virtual void Process();
-  virtual void ProcessTraces();
-
- protected:
-  // Create a new trace.
-  virtual scoped_refptr<Trace> CreateTrace(const std::string& name);
-
-  const unsigned char* gpu_trace_srv_category;
-  const unsigned char* gpu_trace_dev_category;
-
- protected:
-  void IssueProcessTask();
-
-  std::vector<TraceMarker> markers_;
-  std::deque<scoped_refptr<Trace> > traces_;
-
-  bool gpu_executing_;
-  bool process_posted_;
-
-  DISALLOW_COPY_AND_ASSIGN(GPUTracerImpl);
-};
-
-class GPUTracerARBTimerQuery : public GPUTracerImpl {
- public:
-  explicit GPUTracerARBTimerQuery(gles2::GLES2Decoder* decoder);
-  virtual ~GPUTracerARBTimerQuery();
-
-  // Implementation of GPUTracerImpl
-  virtual void ProcessTraces() OVERRIDE;
-
- protected:
-  // Implementation of GPUTracerImpl.
-  virtual bool BeginDecoding() OVERRIDE;
-  virtual bool EndDecoding() OVERRIDE;
-  virtual scoped_refptr<Trace> CreateTrace(const std::string& name) OVERRIDE;
-  virtual void CalculateTimerOffset() OVERRIDE;
-
-  scoped_refptr<Outputter> outputter_;
-
-  bool gpu_timing_synced_;
-  int64 timer_offset_;
-
-  gles2::GLES2Decoder* decoder_;
-
-  DISALLOW_COPY_AND_ASSIGN(GPUTracerARBTimerQuery);
-};
-
-bool Trace::IsProcessable() { return true; }
-
-const std::string& Trace::name() { return name_; }
-
-GLARBTimerTrace::GLARBTimerTrace(scoped_refptr<Outputter> outputter,
-                                 const std::string& name,
-                                 int64 offset)
-    : Trace(name),
-      outputter_(outputter),
-      offset_(offset),
-      start_time_(0),
-      end_time_(0),
-      end_requested_(false) {
-  glGenQueries(2, queries_);
 }
 
-GLARBTimerTrace::~GLARBTimerTrace() { glDeleteQueries(2, queries_); }
-
-void GLARBTimerTrace::Start() {
-  TRACE_EVENT_COPY_ASYNC_BEGIN0(
-      TRACE_DISABLED_BY_DEFAULT("gpu.service"), name().c_str(), this);
-  glQueryCounter(queries_[0], GL_TIMESTAMP);
+GPUTracer::~GPUTracer() {
 }
 
-void GLARBTimerTrace::End() {
-  glQueryCounter(queries_[1], GL_TIMESTAMP);
-  end_requested_ = true;
-  TRACE_EVENT_COPY_ASYNC_END0(
-      TRACE_DISABLED_BY_DEFAULT("gpu.service"), name().c_str(), this);
-}
-
-bool GLARBTimerTrace::IsAvailable() {
-  if (!end_requested_)
-    return false;
-
-  GLint done = 0;
-  glGetQueryObjectiv(queries_[1], GL_QUERY_RESULT_AVAILABLE, &done);
-  return !!done;
-}
-
-void GLARBTimerTrace::Process() {
-  DCHECK(IsAvailable());
-
-  GLuint64 timestamp;
-
-  // TODO(dsinclair): It's possible for the timer to wrap during the start/end.
-  // We need to detect if the end is less then the start and correct for the
-  // wrapping.
-  glGetQueryObjectui64v(queries_[0], GL_QUERY_RESULT, &timestamp);
-  start_time_ = (timestamp / base::Time::kNanosecondsPerMicrosecond) + offset_;
-
-  glGetQueryObjectui64v(queries_[1], GL_QUERY_RESULT, &timestamp);
-  end_time_ = (timestamp / base::Time::kNanosecondsPerMicrosecond) + offset_;
-
-  glDeleteQueries(2, queries_);
-  outputter_->Trace(name(), start_time_, end_time_);
-}
+bool GPUTracer::BeginDecoding() {
+  if (enabled_) {
+    if (*gpu_trace_dev_category) {
+      // Make sure timing is synced before tracing
+      if (!gpu_timing_synced_) {
+        CalculateTimerOffset();
+        gpu_timing_synced_ = true;
+      }
+    } else {
+      // If GPU device category is off, invalidate timing sync
+      gpu_timing_synced_ = false;
+    }
+  }
 
-bool GPUTracerImpl::BeginDecoding() {
   if (gpu_executing_)
     return false;
 
@@ -221,150 +116,144 @@ bool GPUTracerImpl::BeginDecoding() {
 
   if (IsTracing()) {
     // Begin a Trace for all active markers
-    for (size_t i = 0; i < markers_.size(); i++) {
-      markers_[i].trace_ = CreateTrace(markers_[i].name_);
-      markers_[i].trace_->Start();
+    for (int n = 0; n < NUM_TRACER_SOURCES; n++) {
+      for (size_t i = 0; i < markers_[n].size(); i++) {
+        markers_[n][i].trace_ = CreateTrace(markers_[n][i].name_);
+        markers_[n][i].trace_->Start();
+      }
     }
   }
   return true;
 }
 
-bool GPUTracerImpl::EndDecoding() {
+bool GPUTracer::EndDecoding() {
   if (!gpu_executing_)
     return false;
 
   // End Trace for all active markers
   if (IsTracing()) {
-    for (size_t i = 0; i < markers_.size(); i++) {
-      if (markers_[i].trace_) {
-        markers_[i].trace_->End();
-        if (markers_[i].trace_->IsProcessable())
-          traces_.push_back(markers_[i].trace_);
-        markers_[i].trace_ = 0;
+    for (int n = 0; n < NUM_TRACER_SOURCES; n++) {
+      for (size_t i = 0; i < markers_[n].size(); i++) {
+        if (markers_[n][i].trace_) {
+          markers_[n][i].trace_->End();
+          if (markers_[n][i].trace_->IsProcessable())
+            traces_.push_back(markers_[n][i].trace_);
+          markers_[n][i].trace_ = 0;
+        }
       }
     }
     IssueProcessTask();
   }
 
   gpu_executing_ = false;
+
+  // NOTE(vmiura_: glFlush() here can help give better trace results,

[vmiura, 2014/07/29 23:45:30]

nit: NOTE(vmiura)

[David Yen, 2014/08/04 22:20:05]

Done.

+  // but it distorts the normal device behavior.
   return true;
 }
 
-bool GPUTracerImpl::Begin(const std::string& name, GpuTracerSource source) {
+bool GPUTracer::Begin(const std::string& name, GpuTracerSource source) {
   if (!gpu_executing_)
     return false;
 
+  assert(source >= 0 && source < NUM_TRACER_SOURCES);

[vmiura, 2014/07/29 23:45:30]

Please use DCHECK instead of assert here and below.

[David Yen, 2014/08/04 22:20:05]

Done.

+
   // Push new marker from given 'source'
-  markers_.push_back(TraceMarker(name, source));
+  last_tracer_source_ = source;
+  markers_[source].push_back(TraceMarker(name));
 
   // Create trace
   if (IsTracing()) {
     scoped_refptr<Trace> trace = CreateTrace(name);
     trace->Start();
-    markers_.back().trace_ = trace;
+    markers_[source].back().trace_ = trace;
   }
+
   return true;
 }
 
-bool GPUTracerImpl::End(GpuTracerSource source) {
+bool GPUTracer::End(GpuTracerSource source) {
   if (!gpu_executing_)
     return false;
 
+  assert(source >= 0 && source < NUM_TRACER_SOURCES);
+
   // Pop last marker with matching 'source'
-  for (int i = markers_.size() - 1; i >= 0; i--) {
-    if (markers_[i].source_ == source) {
-      // End trace
-      if (IsTracing()) {
-        scoped_refptr<Trace> trace = markers_[i].trace_;
-        if (trace) {
-          trace->End();
-          if (trace->IsProcessable())
-            traces_.push_back(trace);
-          IssueProcessTask();
-        }
+  if (!markers_[source].empty()) {
+    if (IsTracing()) {
+      scoped_refptr<Trace> trace = markers_[source].back().trace_;
+      if (trace) {
+        trace->End();
+        if (trace->IsProcessable())
+          traces_.push_back(trace);
+        IssueProcessTask();
       }
-
-      markers_.erase(markers_.begin() + i);
-      return true;
     }
+
+    markers_[source].pop_back();
+    return true;
   }
   return false;
 }
 
-void GPUTracerImpl::Process() {
-  process_posted_ = false;
-  ProcessTraces();
-  IssueProcessTask();
+bool GPUTracer::IsTracing() {
+  return (*gpu_trace_srv_category != 0) || (*gpu_trace_dev_category != 0);
 }
 
-void GPUTracerImpl::ProcessTraces() {
-  while (!traces_.empty() && traces_.front()->IsAvailable()) {
-    traces_.front()->Process();
-    traces_.pop_front();
+const std::string& GPUTracer::CurrentName() const {
+  if (last_tracer_source_ >= 0 &&
+      last_tracer_source_ < NUM_TRACER_SOURCES &&
+      !markers_[last_tracer_source_].empty()) {
+    return markers_[last_tracer_source_].back().name_;
   }
+  return base::EmptyString();
 }
 
-const std::string& GPUTracerImpl::CurrentName() const {
-  if (markers_.empty())
-    return base::EmptyString();
-  return markers_.back().name_;
+void GPUTracer::CalculateTimerOffset() {
+  if (enabled_) {
+    TRACE_EVENT0("gpu", "GPUTracer::CalculateTimerOffset");
+
+    // NOTE(vmiura): It would be better to use glGetInteger64v, however
+    // it's not available everywhere.
+    GLuint64 gl_now = 0;
+    GLuint query;
+    glFinish();
+    glGenQueries(1, &query);
+    glQueryCounter(query, GL_TIMESTAMP);
+    glFinish();
+    glGetQueryObjectui64v(query, GL_QUERY_RESULT, &gl_now);
+    base::TimeTicks system_now = base::TimeTicks::NowFromSystemTraceTime();
+
+    gl_now /= base::Time::kNanosecondsPerMicrosecond;
+    timer_offset_ = system_now.ToInternalValue() - gl_now;
+    glDeleteQueries(1, &query);
+  }
 }
 
-scoped_refptr<Trace> GPUTracerImpl::CreateTrace(const std::string& name) {
+scoped_refptr<Trace> GPUTracer::CreateTrace(const std::string& name) {
+  if (enabled_) {
+    if (*gpu_trace_dev_category)
+      return new GLARBTimerTrace(outputter_, name, timer_offset_);
+  }
   return new NoopTrace(name);
 }
 
-void GPUTracerImpl::IssueProcessTask() {
-  if (traces_.empty() || process_posted_)
-    return;
-
-  process_posted_ = true;
-  base::MessageLoop::current()->PostDelayedTask(
-      FROM_HERE,
-      base::Bind(&GPUTracerImpl::Process, base::AsWeakPtr(this)),
-      base::TimeDelta::FromMilliseconds(kProcessInterval));
-}
-
-GPUTracerARBTimerQuery::GPUTracerARBTimerQuery(gles2::GLES2Decoder* decoder)
-    : timer_offset_(0), decoder_(decoder) {
-  outputter_ = TraceOutputter::Create("GL_ARB_timer_query");
-}
-
-GPUTracerARBTimerQuery::~GPUTracerARBTimerQuery() {
-}
-
-scoped_refptr<Trace> GPUTracerARBTimerQuery::CreateTrace(
-    const std::string& name) {
-  if (*gpu_trace_dev_category)
-    return new GLARBTimerTrace(outputter_, name, timer_offset_);
-  return GPUTracerImpl::CreateTrace(name);
+void GPUTracer::Process() {
+  process_posted_ = false;
+  ProcessTraces();
+  IssueProcessTask();
 }
 
-bool GPUTracerARBTimerQuery::BeginDecoding() {
-  if (*gpu_trace_dev_category) {
-    // Make sure timing is synced before tracing
-    if (!gpu_timing_synced_) {
-      CalculateTimerOffset();
-      gpu_timing_synced_ = true;
+void GPUTracer::ProcessTraces() {
+  if (!enabled_) {
+    while (!traces_.empty() && traces_.front()->IsAvailable()) {
+      traces_.front()->Process();
+      traces_.pop_front();
     }
-  } else {
-    // If GPU device category is off, invalidate timing sync
-    gpu_timing_synced_ = false;
+    return;
   }
 
-  return GPUTracerImpl::BeginDecoding();
-}
-
-bool GPUTracerARBTimerQuery::EndDecoding() {
-  bool ret = GPUTracerImpl::EndDecoding();
-
-  // NOTE(vmiura_: glFlush() here can help give better trace results,
-  // but it distorts the normal device behavior.
-  return ret;
-}
-
-void GPUTracerARBTimerQuery::ProcessTraces() {
-  TRACE_EVENT0("gpu", "GPUTracerARBTimerQuery::ProcessTraces");
+  TRACE_EVENT0("gpu", "GPUTracer::ProcessTraces");
 
   // Make owning decoder's GL context current
   if (!decoder_->MakeCurrent()) {
@@ -384,34 +273,73 @@ void GPUTracerARBTimerQuery::ProcessTraces() {
     traces_.clear();
 }
 
-void GPUTracerARBTimerQuery::CalculateTimerOffset() {
-  TRACE_EVENT0("gpu", "GPUTracerARBTimerQuery::CalculateTimerOffset");
-
-  // NOTE(vmiura): It would be better to use glGetInteger64v, however
-  // it's not available everywhere.
-  GLuint64 gl_now = 0;
-  GLuint query;
-  glFinish();
-  glGenQueries(1, &query);
-  glQueryCounter(query, GL_TIMESTAMP);
-  glFinish();
-  glGetQueryObjectui64v(query, GL_QUERY_RESULT, &gl_now);
-  base::TimeTicks system_now = base::TimeTicks::NowFromSystemTraceTime();
-
-  gl_now /= base::Time::kNanosecondsPerMicrosecond;
-  timer_offset_ = system_now.ToInternalValue() - gl_now;
-  glDeleteQueries(1, &query);
+void GPUTracer::IssueProcessTask() {
+  if (traces_.empty() || process_posted_)
+    return;
+
+  process_posted_ = true;
+  base::MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&GPUTracer::Process, base::AsWeakPtr(this)),
+      base::TimeDelta::FromMilliseconds(kProcessInterval));
 }
 
-GPUTracer::GPUTracer() {}
+bool Trace::IsProcessable() { return true; }
 
-GPUTracer::~GPUTracer() {}
+const std::string& Trace::name() { return name_; }
 
-scoped_ptr<GPUTracer> GPUTracer::Create(gles2::GLES2Decoder* decoder) {
-  if (gfx::g_driver_gl.ext.b_GL_ARB_timer_query) {
-    return scoped_ptr<GPUTracer>(new GPUTracerARBTimerQuery(decoder));
-  }
-  return scoped_ptr<GPUTracer>(new GPUTracerImpl());
+GLARBTimerTrace::GLARBTimerTrace(scoped_refptr<Outputter> outputter,
+                                 const std::string& name,
+                                 int64 offset)
+    : Trace(name),
+      outputter_(outputter),
+      offset_(offset),
+      start_time_(0),
+      end_time_(0),
+      end_requested_(false) {
+  glGenQueries(2, queries_);
+}
+
+GLARBTimerTrace::~GLARBTimerTrace() { glDeleteQueries(2, queries_); }
+
+void GLARBTimerTrace::Start() {
+  TRACE_EVENT_COPY_ASYNC_BEGIN0(
+      TRACE_DISABLED_BY_DEFAULT("gpu.service"), name().c_str(), this);
+  glQueryCounter(queries_[0], GL_TIMESTAMP);
+}
+
+void GLARBTimerTrace::End() {
+  glQueryCounter(queries_[1], GL_TIMESTAMP);
+  end_requested_ = true;
+  TRACE_EVENT_COPY_ASYNC_END0(
+      TRACE_DISABLED_BY_DEFAULT("gpu.service"), name().c_str(), this);
+}
+
+bool GLARBTimerTrace::IsAvailable() {
+  if (!end_requested_)
+    return false;
+
+  GLint done = 0;
+  glGetQueryObjectiv(queries_[1], GL_QUERY_RESULT_AVAILABLE, &done);
+  return !!done;
+}
+
+void GLARBTimerTrace::Process() {
+  DCHECK(IsAvailable());
+
+  GLuint64 timestamp;
+
+  // TODO(dsinclair): It's possible for the timer to wrap during the start/end.
+  // We need to detect if the end is less then the start and correct for the
+  // wrapping.
+  glGetQueryObjectui64v(queries_[0], GL_QUERY_RESULT, &timestamp);
+  start_time_ = (timestamp / base::Time::kNanosecondsPerMicrosecond) + offset_;
+
+  glGetQueryObjectui64v(queries_[1], GL_QUERY_RESULT, &timestamp);
+  end_time_ = (timestamp / base::Time::kNanosecondsPerMicrosecond) + offset_;
+
+  glDeleteQueries(2, queries_);
+  outputter_->Trace(name(), start_time_, end_time_);
 }
 
 }  // namespace gles2