Skip to content

Commit

Permalink
[cpu-profiler] Move SetIdle() to v8::Isolate
Browse files Browse the repository at this point in the history
The VM state is a property of the isolate, not the CPU profiler.
Having to create a v8::CpuProfiler instance in order to change
the property is somewhat inefficient.

See nodejs/node#18039 and
nodejs/node#18534 for context.

Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I70e31deca6529bccc05a0f4ed500ee268fb63cb8
Reviewed-on: https://chromium-review.googlesource.com/900622
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51779}
  • Loading branch information
hashseed authored and Commit Bot committed Mar 7, 2018
1 parent e885f8a commit 308d4e2
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 43 deletions.
3 changes: 2 additions & 1 deletion include/v8-profiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,8 @@ class V8_EXPORT CpuProfiler {
/**
* Tells the profiler whether the embedder is idle.
*/
void SetIdle(bool is_idle);
V8_DEPRECATED("Use Isolate::SetIdle(bool) instead.",
void SetIdle(bool is_idle));

private:
CpuProfiler();
Expand Down
5 changes: 5 additions & 0 deletions include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -7283,6 +7283,11 @@ class V8_EXPORT Isolate {
V8_DEPRECATED("CpuProfiler should be created with CpuProfiler::New call.",
CpuProfiler* GetCpuProfiler());

/**
* Tells the CPU profiler whether the embedder is idle.
*/
void SetIdle(bool is_idle);

/** Returns true if this isolate has a current context. */
bool InContext();

Expand Down
14 changes: 5 additions & 9 deletions src/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8110,6 +8110,10 @@ CpuProfiler* Isolate::GetCpuProfiler() {
return reinterpret_cast<CpuProfiler*>(cpu_profiler);
}

void Isolate::SetIdle(bool is_idle) {
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
isolate->SetIdle(is_idle);
}

bool Isolate::InContext() {
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
Expand Down Expand Up @@ -10126,15 +10130,7 @@ CpuProfile* CpuProfiler::StopProfiling(Local<String> title) {
void CpuProfiler::SetIdle(bool is_idle) {
i::CpuProfiler* profiler = reinterpret_cast<i::CpuProfiler*>(this);
i::Isolate* isolate = profiler->isolate();
if (!isolate->is_profiling()) return;
v8::StateTag state = isolate->current_vm_state();
DCHECK(state == v8::EXTERNAL || state == v8::IDLE);
if (isolate->js_entry_sp() != nullptr) return;
if (is_idle) {
isolate->set_current_vm_state(v8::IDLE);
} else if (state == v8::IDLE) {
isolate->set_current_vm_state(v8::EXTERNAL);
}
isolate->SetIdle(is_idle);
}


Expand Down
16 changes: 2 additions & 14 deletions src/inspector/v8-inspector-impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -234,21 +234,9 @@ void V8InspectorImpl::resetContextGroup(int contextGroupId) {
m_debugger->wasmTranslation()->Clear();
}

void V8InspectorImpl::idleStarted() {
for (auto& it : m_sessions) {
for (auto& it2 : it.second) {
if (it2.second->profilerAgent()->idleStarted()) return;
}
}
}
void V8InspectorImpl::idleStarted() { m_isolate->SetIdle(true); }

void V8InspectorImpl::idleFinished() {
for (auto& it : m_sessions) {
for (auto& it2 : it.second) {
if (it2.second->profilerAgent()->idleFinished()) return;
}
}
}
void V8InspectorImpl::idleFinished() { m_isolate->SetIdle(false); }

unsigned V8InspectorImpl::exceptionThrown(
v8::Local<v8::Context> context, const StringView& message,
Expand Down
13 changes: 0 additions & 13 deletions src/inspector/v8-profiler-agent-impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,6 @@ void V8ProfilerAgentImpl::startProfiling(const String16& title) {
if (!m_startedProfilesCount) {
DCHECK(!m_profiler);
m_profiler = v8::CpuProfiler::New(m_isolate);
m_profiler->SetIdle(m_idle);
int interval =
m_state->integerProperty(ProfilerAgentState::samplingInterval, 0);
if (interval) m_profiler->SetSamplingInterval(interval);
Expand All @@ -503,16 +502,4 @@ std::unique_ptr<protocol::Profiler::Profile> V8ProfilerAgentImpl::stopProfiling(
return result;
}

bool V8ProfilerAgentImpl::idleStarted() {
m_idle = true;
if (m_profiler) m_profiler->SetIdle(m_idle);
return m_profiler;
}

bool V8ProfilerAgentImpl::idleFinished() {
m_idle = false;
if (m_profiler) m_profiler->SetIdle(m_idle);
return m_profiler;
}

} // namespace v8_inspector
4 changes: 0 additions & 4 deletions src/inspector/v8-profiler-agent-impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ class V8ProfilerAgentImpl : public protocol::Profiler::Backend {
void consoleProfile(const String16& title);
void consoleProfileEnd(const String16& title);

bool idleStarted();
bool idleFinished();

private:
String16 nextProfileId();

Expand All @@ -77,7 +74,6 @@ class V8ProfilerAgentImpl : public protocol::Profiler::Backend {
class ProfileDescriptor;
std::vector<ProfileDescriptor> m_startedProfiles;
String16 m_frontendInitiatedProfileId;
bool m_idle = false;
int m_startedProfilesCount = 0;

DISALLOW_COPY_AND_ASSIGN(V8ProfilerAgentImpl);
Expand Down
12 changes: 12 additions & 0 deletions src/isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4038,6 +4038,18 @@ void Isolate::PrintWithTimestamp(const char* format, ...) {
va_end(arguments);
}

void Isolate::SetIdle(bool is_idle) {
if (!is_profiling()) return;
StateTag state = current_vm_state();
DCHECK(state == EXTERNAL || state == IDLE);
if (js_entry_sp() != nullptr) return;
if (is_idle) {
set_current_vm_state(IDLE);
} else if (state == IDLE) {
set_current_vm_state(EXTERNAL);
}
}

bool StackLimitCheck::JsHasOverflowed(uintptr_t gap) const {
StackGuard* stack_guard = isolate_->stack_guard();
#ifdef USE_SIMULATOR
Expand Down
2 changes: 2 additions & 0 deletions src/isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,8 @@ class Isolate {
top_backup_incumbent_scope_ = top_backup_incumbent_scope;
}

void SetIdle(bool is_idle);

protected:
explicit Isolate(bool enable_serializer);
bool IsArrayOrObjectOrStringPrototype(Object* object);
Expand Down
4 changes: 2 additions & 2 deletions test/cctest/test-cpu-profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1651,11 +1651,11 @@ TEST(IdleTime) {
reinterpret_cast<i::CpuProfiler*>(cpu_profiler)->processor();

processor->AddCurrentStack(isolate, true);
cpu_profiler->SetIdle(true);
isolate->SetIdle(true);
for (int i = 0; i < 3; i++) {
processor->AddCurrentStack(isolate, true);
}
cpu_profiler->SetIdle(false);
isolate->SetIdle(false);
processor->AddCurrentStack(isolate, true);

v8::CpuProfile* profile = cpu_profiler->StopProfiling(profile_name);
Expand Down

0 comments on commit 308d4e2

Please sign in to comment.