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

Jump to bottom

src: use stack-allocated Environment instances #7090

Merged
merged 2 commits into from
Jun 2, 2016
Merged

src: use stack-allocated Environment instances #7090

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
58cec4e
src: move env init logic into Environment class
bnoordhuis Jun 1, 2016
aac79df
src: use stack-allocated Environment instances
bnoordhuis Jun 1, 2016
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
27 changes: 10 additions & 17 deletions src/debug-agent.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,31 +174,24 @@ void Agent::WorkerRun() {
Local<Context> context = Context::New(isolate);

Context::Scope context_scope(context);
Environment* env = CreateEnvironment(
&isolate_data,
context,
arraysize(argv),
argv,
arraysize(argv),
argv);
Environment env(&isolate_data, context);

child_env_ = env;
const bool start_profiler_idle_notifier = false;
env.Start(arraysize(argv), argv,
arraysize(argv), argv,
start_profiler_idle_notifier);

child_env_ = &env;

// Expose API
InitAdaptor(env);
LoadEnvironment(env);
InitAdaptor(&env);
LoadEnvironment(&env);

CHECK_EQ(&child_loop_, env->event_loop());
CHECK_EQ(&child_loop_, env.event_loop());
uv_run(&child_loop_, UV_RUN_DEFAULT);

// Clean-up peristent
api_.Reset();

// Clean-up all running handles
env->CleanupHandles();

env->Dispose();
env = nullptr;
}
isolate->Dispose();
}
Expand Down
51 changes: 11 additions & 40 deletions src/env-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,6 @@ inline void Environment::TickInfo::set_index(uint32_t value) {
fields_[kIndex] = value;
}

inline Environment* Environment::New(IsolateData* isolate_data,
v8::Local<v8::Context> context) {
Environment* env = new Environment(isolate_data, context);
env->AssignToContext(context);
return env;
}

inline void Environment::AssignToContext(v8::Local<v8::Context> context) {
context->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex, this);
}
Expand Down Expand Up @@ -184,6 +177,7 @@ inline Environment::Environment(IsolateData* isolate_data,
#if HAVE_INSPECTOR
inspector_agent_(this),
#endif
handle_cleanup_waiting_(0),
http_parser_buffer_(nullptr),
context_(context->GetIsolate(), context) {
// We'll be creating new objects so make sure we've entered the context.
Expand All @@ -200,24 +194,12 @@ inline Environment::Environment(IsolateData* isolate_data,
set_generic_internal_field_template(obj);

RB_INIT(&cares_task_list_);
handle_cleanup_waiting_ = 0;
AssignToContext(context);
}

inline Environment::~Environment() {
v8::HandleScope handle_scope(isolate());

context()->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex,
nullptr);
#define V(PropertyName, TypeName) PropertyName ## _.Reset();
ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)
#undef V

delete[] heap_statistics_buffer_;
delete[] heap_space_statistics_buffer_;
delete[] http_parser_buffer_;
}

inline void Environment::CleanupHandles() {
while (HandleCleanup* hc = handle_cleanup_queue_.PopFront()) {
handle_cleanup_waiting_++;
hc->cb_(this, hc->handle_, hc->arg_);
Expand All @@ -226,10 +208,16 @@ inline void Environment::CleanupHandles() {

while (handle_cleanup_waiting_ != 0)
uv_run(event_loop(), UV_RUN_ONCE);
}

inline void Environment::Dispose() {
delete this;
context()->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex,
nullptr);
#define V(PropertyName, TypeName) PropertyName ## _.Reset();
ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)
#undef V

delete[] heap_statistics_buffer_;
delete[] heap_space_statistics_buffer_;
delete[] http_parser_buffer_;
}

inline v8::Isolate* Environment::isolate() const {
Expand Down Expand Up @@ -260,23 +248,6 @@ inline uv_idle_t* Environment::immediate_idle_handle() {
return &immediate_idle_handle_;
}

inline Environment* Environment::from_idle_prepare_handle(
uv_prepare_t* handle) {
return ContainerOf(&Environment::idle_prepare_handle_, handle);
}

inline uv_prepare_t* Environment::idle_prepare_handle() {
return &idle_prepare_handle_;
}

inline Environment* Environment::from_idle_check_handle(uv_check_t* handle) {
return ContainerOf(&Environment::idle_check_handle_, handle);
}

inline uv_check_t* Environment::idle_check_handle() {
return &idle_check_handle_;
}

inline void Environment::RegisterHandleCleanup(uv_handle_t* handle,
HandleCleanupCb cb,
void *arg) {
Expand Down
90 changes: 90 additions & 0 deletions src/env.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "env.h"
#include "env-inl.h"
#include "async-wrap.h"
#include "v8.h"
#include "v8-profiler.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Which header is it that includes SetupProcessObject()?

Copy link
Member Author

Choose a reason for hiding this comment

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

node_internals.h, which is included by node.h, which is included by env-inl.h. It's a bit roundabout, I can include it directly.

Copy link
Contributor

@trevnorris trevnorris Jun 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

doesn't worry me. just couldn't see how the header was reaching here.


#if defined(_MSC_VER)
#define getpid GetCurrentProcessId
Expand All @@ -12,13 +14,101 @@

namespace node {

using v8::Context;
using v8::FunctionTemplate;
using v8::HandleScope;
using v8::Local;
using v8::Message;
using v8::StackFrame;
using v8::StackTrace;
using v8::Value;

void Environment::Start(int argc,
const char* const* argv,
int exec_argc,
const char* const* exec_argv,
bool start_profiler_idle_notifier) {
HandleScope handle_scope(isolate());
Context::Scope context_scope(context());

isolate()->SetAutorunMicrotasks(false);

uv_check_init(event_loop(), immediate_check_handle());
uv_unref(reinterpret_cast<uv_handle_t*>(immediate_check_handle()));

uv_idle_init(event_loop(), immediate_idle_handle());

// Inform V8's CPU profiler when we're idle. The profiler is sampling-based
// but not all samples are created equal; mark the wall clock time spent in
// epoll_wait() and friends so profiling tools can filter it out. The samples
// still end up in v8.log but with state=IDLE rather than state=EXTERNAL.
// TODO(bnoordhuis) Depends on a libuv implementation detail that we should
// probably fortify in the API contract, namely that the last started prepare
// or check watcher runs first. It's not 100% foolproof; if an add-on starts
// a prepare or check watcher after us, any samples attributed to its callback
// will be recorded with state=IDLE.
uv_prepare_init(event_loop(), &idle_prepare_handle_);
uv_check_init(event_loop(), &idle_check_handle_);
uv_unref(reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_));
uv_unref(reinterpret_cast<uv_handle_t*>(&idle_check_handle_));

auto close_and_finish = [](Environment* env, uv_handle_t* handle, void* arg) {
handle->data = env;

uv_close(handle, [](uv_handle_t* handle) {
static_cast<Environment*>(handle->data)->FinishHandleCleanup(handle);
});
};

RegisterHandleCleanup(
reinterpret_cast<uv_handle_t*>(immediate_check_handle()),
close_and_finish,
nullptr);
RegisterHandleCleanup(
reinterpret_cast<uv_handle_t*>(immediate_idle_handle()),
close_and_finish,
nullptr);
RegisterHandleCleanup(
reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_),
close_and_finish,
nullptr);
RegisterHandleCleanup(
reinterpret_cast<uv_handle_t*>(&idle_check_handle_),
close_and_finish,
nullptr);

if (start_profiler_idle_notifier) {
StartProfilerIdleNotifier();
}

auto process_template = FunctionTemplate::New(isolate());
process_template->SetClassName(FIXED_ONE_BYTE_STRING(isolate(), "process"));

auto process_object =
process_template->GetFunction()->NewInstance(context()).ToLocalChecked();
set_process_object(process_object);

SetupProcessObject(this, argc, argv, exec_argc, exec_argv);
LoadAsyncWrapperInfo(this);
}

void Environment::StartProfilerIdleNotifier() {
uv_prepare_start(&idle_prepare_handle_, [](uv_prepare_t* handle) {
Environment* env = ContainerOf(&Environment::idle_prepare_handle_, handle);
env->isolate()->GetCpuProfiler()->SetIdle(true);
});

uv_check_start(&idle_check_handle_, [](uv_check_t* handle) {
Environment* env = ContainerOf(&Environment::idle_check_handle_, handle);
env->isolate()->GetCpuProfiler()->SetIdle(false);
});
}

void Environment::StopProfilerIdleNotifier() {
uv_prepare_stop(&idle_prepare_handle_);
uv_check_stop(&idle_check_handle_);
}

void Environment::PrintSyncTrace() const {
if (!trace_sync_io_)
return;
Expand Down
26 changes: 11 additions & 15 deletions src/env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,14 +446,19 @@ class Environment {
static inline Environment* GetCurrent(
const v8::PropertyCallbackInfo<T>& info);

// See CreateEnvironment() in src/node.cc.
static inline Environment* New(IsolateData* isolate_data,
v8::Local<v8::Context> context);
inline void CleanupHandles();
inline void Dispose();
inline Environment(IsolateData* isolate_data, v8::Local<v8::Context> context);
inline ~Environment();

void Start(int argc,
const char* const* argv,
int exec_argc,
const char* const* exec_argv,
bool start_profiler_idle_notifier);
void AssignToContext(v8::Local<v8::Context> context);

void StartProfilerIdleNotifier();
void StopProfilerIdleNotifier();

inline v8::Isolate* isolate() const;
inline uv_loop_t* event_loop() const;
inline bool async_wrap_callbacks_enabled() const;
Expand All @@ -464,13 +469,7 @@ class Environment {
inline uv_check_t* immediate_check_handle();
inline uv_idle_t* immediate_idle_handle();

static inline Environment* from_idle_prepare_handle(uv_prepare_t* handle);
inline uv_prepare_t* idle_prepare_handle();

static inline Environment* from_idle_check_handle(uv_check_t* handle);
inline uv_check_t* idle_check_handle();

// Register clean-up cb to be called on env->Dispose()
// Register clean-up cb to be called on environment destruction.
inline void RegisterHandleCleanup(uv_handle_t* handle,
HandleCleanupCb cb,
void *arg);
Expand Down Expand Up @@ -582,9 +581,6 @@ class Environment {
static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX;

private:
inline Environment(IsolateData* isolate_data, v8::Local<v8::Context> context);
inline ~Environment();

v8::Isolate* const isolate_;
IsolateData* const isolate_data_;
uv_check_t immediate_check_handle_;
Expand Down
Loading