Skip to content

Commit

Permalink
Tracing: keep track of agents that are tracing
Browse files Browse the repository at this point in the history
Instead of asking all agents to keep track of whether they are currently
tracing or not and bail out of StopAndFlush if they are not tracing, the
coordinator only sends StopAndFlush to an agent if the agent has successfully
started tracing.

BUG=774728

Change-Id: I79801d5c66a454ee3bb1ea4c6729039e81c61a82
Reviewed-on: https://chromium-review.googlesource.com/721967
Commit-Queue: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509439}(cherry picked from commit 6588f9b)
Reviewed-on: https://chromium-review.googlesource.com/726539
Reviewed-by: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#55}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
  • Loading branch information
chiniforooshan committed Oct 18, 2017
1 parent b2c1eea commit f47ffe1
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 7 deletions.
4 changes: 1 addition & 3 deletions content/browser/tracing/cros_tracing_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ void CrOSTracingAgent::StartTracing(
base::trace_event::TraceConfig trace_config(config);
debug_daemon_ = chromeos::DBusThreadManager::Get()->GetDebugDaemonClient();
if (!trace_config.IsSystraceEnabled() || !debug_daemon_) {
debug_daemon_ = nullptr;
callback.Run(false /* success */);
return;
}
Expand All @@ -63,8 +62,7 @@ void CrOSTracingAgent::StartTracing(
}

void CrOSTracingAgent::StopAndFlush(tracing::mojom::RecorderPtr recorder) {
if (!debug_daemon_)
return;
DCHECK(debug_daemon_);
recorder_ = std::move(recorder);
debug_daemon_->StopAgentTracing(base::BindRepeating(
&CrOSTracingAgent::RecorderProxy, base::Unretained(this)));
Expand Down
3 changes: 1 addition & 2 deletions content/browser/tracing/etw_tracing_agent_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ void EtwTracingAgent::StartTracing(
}

void EtwTracingAgent::StopAndFlush(tracing::mojom::RecorderPtr recorder) {
if (!is_tracing_)
return;
DCHECK(is_tracing_);
// Deactivate kernel tracing.
if (!StopKernelSessionTracing()) {
LOG(FATAL) << "Could not stop system tracing.";
Expand Down
3 changes: 2 additions & 1 deletion services/resource_coordinator/tracing/agent_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ AgentRegistry::AgentEntry::AgentEntry(size_t id,
agent_(std::move(agent)),
label_(label),
type_(type),
supports_explicit_clock_sync_(supports_explicit_clock_sync) {
supports_explicit_clock_sync_(supports_explicit_clock_sync),
is_tracing_(false) {
DCHECK(!label.empty());
agent_.set_connection_error_handler(base::BindRepeating(
&AgentRegistry::AgentEntry::OnConnectionError, base::Unretained(this)));
Expand Down
3 changes: 3 additions & 0 deletions services/resource_coordinator/tracing/agent_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class AgentRegistry : public mojom::AgentRegistry {
bool supports_explicit_clock_sync() const {
return supports_explicit_clock_sync_;
}
bool is_tracing() const { return is_tracing_; }
void set_is_tracing(bool is_tracing) { is_tracing_ = is_tracing; }

private:
void OnConnectionError();
Expand All @@ -59,6 +61,7 @@ class AgentRegistry : public mojom::AgentRegistry {
const mojom::TraceDataType type_;
const bool supports_explicit_clock_sync_;
std::map<const void*, base::OnceClosure> closures_;
bool is_tracing_;

DISALLOW_COPY_AND_ASSIGN(AgentEntry);
};
Expand Down
11 changes: 10 additions & 1 deletion services/resource_coordinator/tracing/coordinator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ void Coordinator::StartTracing(const std::string& config,

void Coordinator::SendStartTracingToAgent(
AgentRegistry::AgentEntry* agent_entry) {
DCHECK(!agent_entry->is_tracing());
agent_entry->AddDisconnectClosure(
&kStartTracingClosureName,
base::BindOnce(&Coordinator::OnTracingStarted, base::Unretained(this),
Expand All @@ -121,6 +122,7 @@ void Coordinator::SendStartTracingToAgent(

void Coordinator::OnTracingStarted(AgentRegistry::AgentEntry* agent_entry,
bool success) {
agent_entry->set_is_tracing(success);
bool removed =
agent_entry->RemoveDisconnectClosure(&kStartTracingClosureName);
DCHECK(removed);
Expand Down Expand Up @@ -169,8 +171,10 @@ void Coordinator::StopAndFlushInternal() {
}

agent_registry_->ForAllAgents([this](AgentRegistry::AgentEntry* agent_entry) {
if (!agent_entry->supports_explicit_clock_sync())
if (!agent_entry->is_tracing() ||
!agent_entry->supports_explicit_clock_sync()) {
return;
}
const std::string sync_id = base::GenerateGUID();
agent_entry->AddDisconnectClosure(
&kRequestClockSyncMarkerClosureName,
Expand Down Expand Up @@ -216,6 +220,8 @@ void Coordinator::StopAndFlushAfterClockSync() {
streaming_label_.clear();

agent_registry_->ForAllAgents([this](AgentRegistry::AgentEntry* agent_entry) {
if (!agent_entry->is_tracing())
return;
mojom::RecorderPtr ptr;
recorders_[agent_entry->label()].insert(base::MakeUnique<Recorder>(
MakeRequest(&ptr), agent_entry->type(),
Expand Down Expand Up @@ -372,6 +378,9 @@ void Coordinator::OnFlushDone() {
recorders_.clear();
stream_.reset();
base::ResetAndReturn(&stop_and_flush_callback_).Run(std::move(metadata_));
agent_registry_->ForAllAgents([this](AgentRegistry::AgentEntry* agent_entry) {
agent_entry->set_is_tracing(false);
});
is_tracing_ = false;
}

Expand Down

0 comments on commit f47ffe1

Please sign in to comment.