Skip to content

Commit

Permalink
Simplified GPU Tracer by removing base Trace class.
Browse files Browse the repository at this point in the history 
First step in simplifying the GPU Tracer is by removing
the parent Trace class. Because the Trace class is created
and managed directly by the GPUTracer class, there is no
reason to have a separate base Trace class that can be
overridden through virtual functions.

BUG= https://code.google.com/p/chromium/issues/detail?id=397294
TEST= trybots

Review URL: https://codereview.chromium.org/420573002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286331 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
dyen@chromium.org committed Jul 29, 2014
1 parent 50610d4 commit fde50c7
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 127 deletions.
171 changes: 84 additions & 87 deletions gpu/command_buffer/service/gpu_tracer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,36 +51,90 @@ void TraceOutputter::Trace(const std::string& name,
++local_trace_id_;
}

class NoopTrace : public Trace {
public:
explicit NoopTrace(const std::string& name) : Trace(name) {}
GPUTrace::GPUTrace(const std::string& name)
: name_(name),
outputter_(NULL),
offset_(0),
end_time_(0),
end_requested_(false),
enabled_(false) {
}

GPUTrace::GPUTrace(scoped_refptr<Outputter> outputter,
const std::string& name,
int64 offset)
: name_(name),
outputter_(outputter),
offset_(offset),
start_time_(0),
end_time_(0),
end_requested_(false),
enabled_(true) {
glGenQueries(2, queries_);
}

GPUTrace::~GPUTrace() {
if (enabled_)
glDeleteQueries(2, queries_);
}

// Implementation of Tracer
virtual void Start() OVERRIDE {
TRACE_EVENT_COPY_ASYNC_BEGIN0(
TRACE_DISABLED_BY_DEFAULT("gpu.service"), name().c_str(), this);
void GPUTrace::Start() {
TRACE_EVENT_COPY_ASYNC_BEGIN0(
TRACE_DISABLED_BY_DEFAULT("gpu.service"), name().c_str(), this);
if (enabled_) {
glQueryCounter(queries_[0], GL_TIMESTAMP);
}
virtual void End() OVERRIDE {
TRACE_EVENT_COPY_ASYNC_END0(
TRACE_DISABLED_BY_DEFAULT("gpu.service"), name().c_str(), this);
}

void GPUTrace::End() {
if (enabled_) {
glQueryCounter(queries_[1], GL_TIMESTAMP);
end_requested_ = true;
}
virtual bool IsAvailable() OVERRIDE { return true; }
virtual bool IsProcessable() OVERRIDE { return false; }
virtual void Process() OVERRIDE {}

private:
virtual ~NoopTrace() {}
TRACE_EVENT_COPY_ASYNC_END0(
TRACE_DISABLED_BY_DEFAULT("gpu.service"), name().c_str(), this);
}

DISALLOW_COPY_AND_ASSIGN(NoopTrace);
};
bool GPUTrace::IsAvailable() {
if (!enabled_)
return true;
else if (!end_requested_)
return false;

GLint done = 0;
glGetQueryObjectiv(queries_[1], GL_QUERY_RESULT_AVAILABLE, &done);
return !!done;
}

void GPUTrace::Process() {
if (!enabled_)
return;

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_);
}

struct TraceMarker {
TraceMarker(const std::string& name, GpuTracerSource source)
: name_(name), source_(source) {}

std::string name_;
GpuTracerSource source_;
scoped_refptr<Trace> trace_;
scoped_refptr<GPUTrace> trace_;
};

class GPUTracerImpl
Expand Down Expand Up @@ -113,7 +167,7 @@ class GPUTracerImpl

protected:
// Create a new trace.
virtual scoped_refptr<Trace> CreateTrace(const std::string& name);
virtual scoped_refptr<GPUTrace> CreateTrace(const std::string& name);

const unsigned char* gpu_trace_srv_category;
const unsigned char* gpu_trace_dev_category;
Expand All @@ -122,7 +176,7 @@ class GPUTracerImpl
void IssueProcessTask();

std::vector<TraceMarker> markers_;
std::deque<scoped_refptr<Trace> > traces_;
std::deque<scoped_refptr<GPUTrace> > traces_;

bool gpu_executing_;
bool process_posted_;
Expand All @@ -142,7 +196,7 @@ class GPUTracerARBTimerQuery : public GPUTracerImpl {
// Implementation of GPUTracerImpl.
virtual bool BeginDecoding() OVERRIDE;
virtual bool EndDecoding() OVERRIDE;
virtual scoped_refptr<Trace> CreateTrace(const std::string& name) OVERRIDE;
virtual scoped_refptr<GPUTrace> CreateTrace(const std::string& name) OVERRIDE;
virtual void CalculateTimerOffset() OVERRIDE;

scoped_refptr<Outputter> outputter_;
Expand All @@ -155,64 +209,6 @@ class GPUTracerARBTimerQuery : public GPUTracerImpl {
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);
}

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 GPUTracerImpl::BeginDecoding() {
if (gpu_executing_)
return false;
Expand All @@ -238,7 +234,7 @@ bool GPUTracerImpl::EndDecoding() {
for (size_t i = 0; i < markers_.size(); i++) {
if (markers_[i].trace_) {
markers_[i].trace_->End();
if (markers_[i].trace_->IsProcessable())
if (markers_[i].trace_->IsEnabled())
traces_.push_back(markers_[i].trace_);
markers_[i].trace_ = 0;
}
Expand All @@ -259,7 +255,7 @@ bool GPUTracerImpl::Begin(const std::string& name, GpuTracerSource source) {

// Create trace
if (IsTracing()) {
scoped_refptr<Trace> trace = CreateTrace(name);
scoped_refptr<GPUTrace> trace = CreateTrace(name);
trace->Start();
markers_.back().trace_ = trace;
}
Expand All @@ -275,10 +271,10 @@ bool GPUTracerImpl::End(GpuTracerSource source) {
if (markers_[i].source_ == source) {
// End trace
if (IsTracing()) {
scoped_refptr<Trace> trace = markers_[i].trace_;
scoped_refptr<GPUTrace> trace = markers_[i].trace_;
if (trace) {
trace->End();
if (trace->IsProcessable())
if (trace->IsEnabled())
traces_.push_back(trace);
IssueProcessTask();
}
Expand Down Expand Up @@ -310,8 +306,9 @@ const std::string& GPUTracerImpl::CurrentName() const {
return markers_.back().name_;
}

scoped_refptr<Trace> GPUTracerImpl::CreateTrace(const std::string& name) {
return new NoopTrace(name);
scoped_refptr<GPUTrace> GPUTracerImpl::CreateTrace(
const std::string& name) {
return new GPUTrace(name);
}

void GPUTracerImpl::IssueProcessTask() {
Expand All @@ -333,10 +330,10 @@ GPUTracerARBTimerQuery::GPUTracerARBTimerQuery(gles2::GLES2Decoder* decoder)
GPUTracerARBTimerQuery::~GPUTracerARBTimerQuery() {
}

scoped_refptr<Trace> GPUTracerARBTimerQuery::CreateTrace(
scoped_refptr<GPUTrace> GPUTracerARBTimerQuery::CreateTrace(
const std::string& name) {
if (*gpu_trace_dev_category)
return new GLARBTimerTrace(outputter_, name, timer_offset_);
return new GPUTrace(outputter_, name, timer_offset_);
return GPUTracerImpl::CreateTrace(name);
}

Expand Down
54 changes: 18 additions & 36 deletions gpu/command_buffer/service/gpu_tracer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,59 +85,41 @@ class TraceOutputter : public Outputter {
DISALLOW_COPY_AND_ASSIGN(TraceOutputter);
};

class GPU_EXPORT Trace : public base::RefCounted<Trace> {
class GPU_EXPORT GPUTrace
: public base::RefCounted<GPUTrace> {
public:
explicit Trace(const std::string& name) : name_(name) {}
explicit GPUTrace(const std::string& name);
GPUTrace(scoped_refptr<Outputter> outputter,
const std::string& name,
int64 offset);

virtual void Start() = 0;
virtual void End() = 0;
bool IsEnabled() { return enabled_; }
const std::string& name() { return name_; }

// True if the the results of this query are available.
virtual bool IsAvailable() = 0;

virtual bool IsProcessable();
virtual void Process() = 0;

virtual const std::string& name();

protected:
virtual ~Trace() {}
void Start();
void End();
bool IsAvailable();
void Process();

private:
friend class base::RefCounted<Trace>;

std::string name_;

DISALLOW_COPY_AND_ASSIGN(Trace);
};

class GPU_EXPORT GLARBTimerTrace : public Trace {
public:
GLARBTimerTrace(scoped_refptr<Outputter> outputter,
const std::string& name,
int64 offset);

// Implementation of Tracer
virtual void Start() OVERRIDE;
virtual void End() OVERRIDE;
virtual bool IsAvailable() OVERRIDE;
virtual void Process() OVERRIDE;

private:
virtual ~GLARBTimerTrace();
~GPUTrace();

void Output();

friend class base::RefCounted<GPUTrace>;

std::string name_;
scoped_refptr<Outputter> outputter_;

int64 offset_;
int64 start_time_;
int64 end_time_;
bool end_requested_;
bool enabled_;

GLuint queries_[2];

DISALLOW_COPY_AND_ASSIGN(GLARBTimerTrace);
DISALLOW_COPY_AND_ASSIGN(GPUTrace);
};

} // namespace gles2
Expand Down
8 changes: 4 additions & 4 deletions gpu/command_buffer/service/gpu_tracer_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class GpuTracerTest : public GpuServiceTest {
}

void SetupTimerQueryMocks() {
// Delegate query APIs used by GLARBTimerTrace to a GlFakeQueries
// Delegate query APIs used by GPUTrace to a GlFakeQueries
EXPECT_CALL(*gl_, GenQueries(_, NotNull())).Times(AtLeast(1)).WillOnce(
Invoke(&gl_fake_queries_, &GlFakeQueries::GenQueries));

Expand Down Expand Up @@ -156,7 +156,7 @@ class GpuTracerTest : public GpuServiceTest {
GlFakeQueries gl_fake_queries_;
};

TEST_F(GpuTracerTest, GLARBTimerTrace) {
TEST_F(GpuTracerTest, GPUTrace) {
// Test basic timer query functionality
{
MockOutputter* outputter = new MockOutputter();
Expand All @@ -179,8 +179,8 @@ TEST_F(GpuTracerTest, GLARBTimerTrace) {
EXPECT_CALL(*outputter,
Trace(trace_name, expect_start_time, expect_end_time));

scoped_refptr<GLARBTimerTrace> trace =
new GLARBTimerTrace(outputter_ref, trace_name, offset_time);
scoped_refptr<GPUTrace> trace =
new GPUTrace(outputter_ref, trace_name, offset_time);

gl_fake_queries_.SetCurrentGLTime(start_timestamp);
trace->Start();
Expand Down

0 comments on commit fde50c7

Please sign in to comment.