Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trace_event: destroy platform before tracing #22938

Closed
wants to merge 2 commits into from
Closed

trace_event: destroy platform before tracing #22938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7139e97
trace_event: destroy platform before tracing
ofrobots Sep 18, 2018
db9b5ff
[squash] lint fix
ofrobots Sep 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,18 @@ static struct {
controller->AddTraceStateObserver(new NodeTraceStateObserver(controller));
tracing::TraceEventHelper::SetTracingController(controller);
StartTracingAgent();
// Tracing must be initialized before platform threads are created.
platform_ = new NodePlatform(thread_pool_size, controller);
V8::InitializePlatform(platform_);
}

void Dispose() {
tracing_agent_.reset(nullptr);
platform_->Shutdown();
delete platform_;
platform_ = nullptr;
// Destroy tracing after the platform (and platform threads) have been
// stopped.
tracing_agent_.reset(nullptr);
}

void DrainVMTasks(Isolate* isolate) {
Expand Down
7 changes: 3 additions & 4 deletions src/node_platform.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,9 @@ int PerIsolatePlatformData::unref() {
NodePlatform::NodePlatform(int thread_pool_size,
TracingController* tracing_controller) {
if (tracing_controller) {
tracing_controller_.reset(tracing_controller);
tracing_controller_ = tracing_controller;
} else {
TracingController* controller = new TracingController();
tracing_controller_.reset(controller);
tracing_controller_ = new TracingController();
}
worker_thread_task_runner_ =
std::make_shared<WorkerThreadsTaskRunner>(thread_pool_size);
Expand Down Expand Up @@ -423,7 +422,7 @@ double NodePlatform::CurrentClockTimeMillis() {
}

TracingController* NodePlatform::GetTracingController() {
return tracing_controller_.get();
return tracing_controller_;
}

template <class T>
Expand Down
2 changes: 1 addition & 1 deletion src/node_platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class NodePlatform : public MultiIsolatePlatform {
std::unordered_map<v8::Isolate*,
std::shared_ptr<PerIsolatePlatformData>> per_isolate_;

std::unique_ptr<v8::TracingController> tracing_controller_;
v8::TracingController* tracing_controller_;
std::shared_ptr<WorkerThreadsTaskRunner> worker_thread_task_runner_;
};

Expand Down
11 changes: 5 additions & 6 deletions src/tracing/agent.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ using v8::platform::tracing::TraceConfig;
using v8::platform::tracing::TraceWriter;
using std::string;

Agent::Agent() {
tracing_controller_ = new TracingController();
Agent::Agent() : tracing_controller_(new TracingController()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the TracingController life is 1:1 with the Agent, make it a member instead of a pointer-to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, the lifetimes aren't perfectly aligned. After this PR lands, I have an intention to refactor & merge the Agent and TracingController concepts into a single structure.

tracing_controller_->Initialize(nullptr);

CHECK_EQ(uv_loop_init(&tracing_loop_), 0);
Expand Down Expand Up @@ -117,7 +116,7 @@ AgentWriterHandle Agent::AddClient(
use_categories = &categories_with_default;
}

ScopedSuspendTracing suspend(tracing_controller_, this);
ScopedSuspendTracing suspend(tracing_controller_.get(), this);
int id = next_writer_id_++;
AsyncTraceWriter* raw = writer.get();
writers_[id] = std::move(writer);
Expand Down Expand Up @@ -157,7 +156,7 @@ void Agent::Disconnect(int client) {
Mutex::ScopedLock lock(initialize_writer_mutex_);
to_be_initialized_.erase(writers_[client].get());
}
ScopedSuspendTracing suspend(tracing_controller_, this);
ScopedSuspendTracing suspend(tracing_controller_.get(), this);
writers_.erase(client);
categories_.erase(client);
}
Expand All @@ -166,13 +165,13 @@ void Agent::Enable(int id, const std::set<std::string>& categories) {
if (categories.empty())
return;

ScopedSuspendTracing suspend(tracing_controller_, this,
ScopedSuspendTracing suspend(tracing_controller_.get(), this,
id != kDefaultHandleId);
categories_[id].insert(categories.begin(), categories.end());
}

void Agent::Disable(int id, const std::set<std::string>& categories) {
ScopedSuspendTracing suspend(tracing_controller_, this,
ScopedSuspendTracing suspend(tracing_controller_.get(), this,
id != kDefaultHandleId);
std::multiset<std::string>& writer_categories = categories_[id];
for (const std::string& category : categories) {
Expand Down
6 changes: 4 additions & 2 deletions src/tracing/agent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ class Agent {
Agent();
~Agent();

TracingController* GetTracingController() { return tracing_controller_; }
TracingController* GetTracingController() {
return tracing_controller_.get();
}

enum UseDefaultCategoryMode {
kUseDefaultCategories,
Expand Down Expand Up @@ -121,7 +123,7 @@ class Agent {
// These maps store the original arguments to AddClient(), by id.
std::unordered_map<int, std::multiset<std::string>> categories_;
std::unordered_map<int, std::unique_ptr<AsyncTraceWriter>> writers_;
TracingController* tracing_controller_ = nullptr;
std::unique_ptr<TracingController> tracing_controller_;

// Variables related to initializing per-event-loop properties of individual
// writers, such as libuv handles.
Expand Down