Skip to content

Commit

Permalink
lib,src: remove post-gc event infrastructure
Browse files Browse the repository at this point in the history 
Remove the 'gc' event from the v8 module and remove the supporting
infrastructure from src/.  It gets the axe because:

1. There are currently no users.  It was originally conceived as
   an upstreamed subset of StrongLoop's strong-agent GC metrics,
   but the strong-agent code base has evolved considerably since
   that time and has no use anymore for what is in core.

2. The implementation is not quite sound.  It calls into JS land
   from inside the GC epilog and that is unsafe.  We could fix
   that by delaying the callback until a safe time but because
   there are no users anyway, removing it is all around easier.

PR-URL: #174
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
@bnoordhuis
bnoordhuis committed Dec 18, 2014
1 parent ebf9f29 commit dab6f68
Show file tree
Hide file tree
Showing 5 changed files with 3 additions and 303 deletions.
26 changes: 2 additions & 24 deletions lib/v8.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,6 @@

'use strict';

var EventEmitter = require('events');
var v8binding = process.binding('v8');

var v8 = module.exports = new EventEmitter();
v8.getHeapStatistics = v8binding.getHeapStatistics;
v8.setFlagsFromString = v8binding.setFlagsFromString;


function emitGC(before, after) {
v8.emit('gc', before, after);
}


v8.on('newListener', function(name) {
if (name === 'gc' && EventEmitter.listenerCount(this, name) === 0) {
v8binding.startGarbageCollectionTracking(emitGC);
}
});


v8.on('removeListener', function(name) {
if (name === 'gc' && EventEmitter.listenerCount(this, name) === 0) {
v8binding.stopGarbageCollectionTracking();
}
});
exports.getHeapStatistics = v8binding.getHeapStatistics;
exports.setFlagsFromString = v8binding.setFlagsFromString;
39 changes: 1 addition & 38 deletions src/env-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,40 +34,6 @@

namespace node {

inline Environment::GCInfo::GCInfo()
: type_(static_cast<v8::GCType>(0)),
flags_(static_cast<v8::GCCallbackFlags>(0)),
timestamp_(0) {
}

inline Environment::GCInfo::GCInfo(v8::Isolate* isolate,
v8::GCType type,
v8::GCCallbackFlags flags,
uint64_t timestamp)
: type_(type),
flags_(flags),
timestamp_(timestamp) {
isolate->GetHeapStatistics(&stats_);
}

inline v8::GCType Environment::GCInfo::type() const {
return type_;
}

inline v8::GCCallbackFlags Environment::GCInfo::flags() const {
return flags_;
}

inline v8::HeapStatistics* Environment::GCInfo::stats() const {
// TODO(bnoordhuis) Const-ify once https://codereview.chromium.org/63693005
// lands and makes it way into a stable release.
return const_cast<v8::HeapStatistics*>(&stats_);
}

inline uint64_t Environment::GCInfo::timestamp() const {
return timestamp_;
}

inline Environment::IsolateData* Environment::IsolateData::Get(
v8::Isolate* isolate) {
return static_cast<IsolateData*>(isolate->GetData(kIsolateSlot));
Expand Down Expand Up @@ -99,9 +65,7 @@ inline Environment::IsolateData::IsolateData(v8::Isolate* isolate,
PropertyName ## _(isolate, FIXED_ONE_BYTE_STRING(isolate, StringValue)),
PER_ISOLATE_STRING_PROPERTIES(V)
#undef V
ref_count_(0) {
QUEUE_INIT(&gc_tracker_queue_);
}
ref_count_(0) {}

inline uv_loop_t* Environment::IsolateData::event_loop() const {
return event_loop_;
Expand Down Expand Up @@ -231,7 +195,6 @@ inline Environment::Environment(v8::Local<v8::Context> context,
set_binding_cache_object(v8::Object::New(isolate()));
set_module_load_list_array(v8::Array::New(isolate()));
RB_INIT(&cares_task_list_);
QUEUE_INIT(&gc_tracker_queue_);
QUEUE_INIT(&req_wrap_queue_);
QUEUE_INIT(&handle_wrap_queue_);
QUEUE_INIT(&handle_cleanup_queue_);
Expand Down
46 changes: 0 additions & 46 deletions src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,6 @@ namespace node {
V(context, v8::Context) \
V(domain_array, v8::Array) \
V(fs_stats_constructor_function, v8::Function) \
V(gc_info_callback_function, v8::Function) \
V(module_load_list_array, v8::Array) \
V(pipe_constructor_template, v8::FunctionTemplate) \
V(process_object, v8::Object) \
Expand Down Expand Up @@ -390,10 +389,6 @@ class Environment {
inline void CleanupHandles();
inline void Dispose();

// Defined in src/node_profiler.cc.
void StartGarbageCollectionTracking(v8::Local<v8::Function> callback);
void StopGarbageCollectionTracking();

void AssignToContext(v8::Local<v8::Context> context);

inline v8::Isolate* isolate() const;
Expand Down Expand Up @@ -494,13 +489,10 @@ class Environment {
private:
static const int kIsolateSlot = NODE_ISOLATE_SLOT;

class GCInfo;
class IsolateData;
inline Environment(v8::Local<v8::Context> context, uv_loop_t* loop);
inline ~Environment();
inline IsolateData* isolate_data() const;
void AfterGarbageCollectionCallback(const GCInfo* before,
const GCInfo* after);

enum ContextEmbedderDataIndex {
kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX
Expand All @@ -520,7 +512,6 @@ class Environment {
ares_task_list cares_task_list_;
bool using_smalloc_alloc_cb_;
bool using_domains_;
QUEUE gc_tracker_queue_;
bool printed_error_;
debugger::Agent debugger_agent_;

Expand All @@ -536,26 +527,6 @@ class Environment {
ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)
#undef V

class GCInfo {
public:
inline GCInfo();
inline GCInfo(v8::Isolate* isolate,
v8::GCType type,
v8::GCCallbackFlags flags,
uint64_t timestamp);
inline v8::GCType type() const;
inline v8::GCCallbackFlags flags() const;
// TODO(bnoordhuis) Const-ify once https://codereview.chromium.org/63693005
// lands and makes it way into a stable release.
inline v8::HeapStatistics* stats() const;
inline uint64_t timestamp() const;
private:
v8::GCType type_;
v8::GCCallbackFlags flags_;
v8::HeapStatistics stats_;
uint64_t timestamp_;
};

// Per-thread, reference-counted singleton.
class IsolateData {
public:
Expand All @@ -564,10 +535,6 @@ class Environment {
inline void Put();
inline uv_loop_t* event_loop() const;

// Defined in src/node_profiler.cc.
void StartGarbageCollectionTracking(Environment* env);
void StopGarbageCollectionTracking(Environment* env);

#define V(PropertyName, StringValue) \
inline v8::Local<v8::String> PropertyName() const;
PER_ISOLATE_STRING_PROPERTIES(V)
Expand All @@ -578,16 +545,6 @@ class Environment {
inline explicit IsolateData(v8::Isolate* isolate, uv_loop_t* loop);
inline v8::Isolate* isolate() const;

// Defined in src/node_profiler.cc.
static void BeforeGarbageCollection(v8::Isolate* isolate,
v8::GCType type,
v8::GCCallbackFlags flags);
static void AfterGarbageCollection(v8::Isolate* isolate,
v8::GCType type,
v8::GCCallbackFlags flags);
void BeforeGarbageCollection(v8::GCType type, v8::GCCallbackFlags flags);
void AfterGarbageCollection(v8::GCType type, v8::GCCallbackFlags flags);

uv_loop_t* const event_loop_;
v8::Isolate* const isolate_;

Expand All @@ -597,9 +554,6 @@ class Environment {
#undef V

unsigned int ref_count_;
QUEUE gc_tracker_queue_;
GCInfo gc_info_before_;
GCInfo gc_info_after_;

DISALLOW_COPY_AND_ASSIGN(IsolateData);
};
Expand Down
149 changes: 0 additions & 149 deletions src/node_v8.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,153 +31,15 @@ namespace node {
using v8::Context;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::GCCallbackFlags;
using v8::GCType;
using v8::Handle;
using v8::HandleScope;
using v8::HeapStatistics;
using v8::Isolate;
using v8::Local;
using v8::Null;
using v8::Number;
using v8::Object;
using v8::String;
using v8::Uint32;
using v8::V8;
using v8::Value;
using v8::kGCTypeAll;
using v8::kGCTypeMarkSweepCompact;
using v8::kGCTypeScavenge;


void Environment::IsolateData::BeforeGarbageCollection(Isolate* isolate,
GCType type,
GCCallbackFlags flags) {
Get(isolate)->BeforeGarbageCollection(type, flags);
}


void Environment::IsolateData::AfterGarbageCollection(Isolate* isolate,
GCType type,
GCCallbackFlags flags) {
Get(isolate)->AfterGarbageCollection(type, flags);
}


void Environment::IsolateData::BeforeGarbageCollection(GCType type,
GCCallbackFlags flags) {
gc_info_before_ = GCInfo(isolate(), type, flags, uv_hrtime());
}


void Environment::IsolateData::AfterGarbageCollection(GCType type,
GCCallbackFlags flags) {
gc_info_after_ = GCInfo(isolate(), type, flags, uv_hrtime());

// The copy upfront and the remove-then-insert is to avoid corrupting the
// list when the callback removes itself from it. QUEUE_FOREACH() is unsafe
// when the list is mutated while being walked.
ASSERT(QUEUE_EMPTY(&gc_tracker_queue_) == false);
QUEUE queue;
QUEUE* q = QUEUE_HEAD(&gc_tracker_queue_);
QUEUE_SPLIT(&gc_tracker_queue_, q, &queue);
while (QUEUE_EMPTY(&queue) == false) {
q = QUEUE_HEAD(&queue);
QUEUE_REMOVE(q);
QUEUE_INSERT_TAIL(&gc_tracker_queue_, q);
Environment* env = ContainerOf(&Environment::gc_tracker_queue_, q);
env->AfterGarbageCollectionCallback(&gc_info_before_, &gc_info_after_);
}
}


void Environment::IsolateData::StartGarbageCollectionTracking(
Environment* env) {
if (QUEUE_EMPTY(&gc_tracker_queue_)) {
isolate()->AddGCPrologueCallback(BeforeGarbageCollection, v8::kGCTypeAll);
isolate()->AddGCEpilogueCallback(AfterGarbageCollection, v8::kGCTypeAll);
}
ASSERT(QUEUE_EMPTY(&env->gc_tracker_queue_) == true);
QUEUE_INSERT_TAIL(&gc_tracker_queue_, &env->gc_tracker_queue_);
}


void Environment::IsolateData::StopGarbageCollectionTracking(Environment* env) {
ASSERT(QUEUE_EMPTY(&env->gc_tracker_queue_) == false);
QUEUE_REMOVE(&env->gc_tracker_queue_);
QUEUE_INIT(&env->gc_tracker_queue_);
if (QUEUE_EMPTY(&gc_tracker_queue_)) {
isolate()->RemoveGCPrologueCallback(BeforeGarbageCollection);
isolate()->RemoveGCEpilogueCallback(AfterGarbageCollection);
}
}


// Considering a memory constrained environment, creating more objects is less
// than ideal
void Environment::AfterGarbageCollectionCallback(const GCInfo* before,
const GCInfo* after) {
HandleScope handle_scope(isolate());
Context::Scope context_scope(context());
Local<Value> argv[] = { Object::New(isolate()), Object::New(isolate()) };
const GCInfo* infov[] = { before, after };
for (unsigned i = 0; i < ARRAY_SIZE(argv); i += 1) {
Local<Object> obj = argv[i].As<Object>();
const GCInfo* info = infov[i];
switch (info->type()) {
case kGCTypeScavenge:
obj->Set(type_string(), scavenge_string());
break;
case kGCTypeMarkSweepCompact:
obj->Set(type_string(), mark_sweep_compact_string());
break;
default:
UNREACHABLE();
}
obj->Set(flags_string(), Uint32::NewFromUnsigned(isolate(), info->flags()));
obj->Set(timestamp_string(), Number::New(isolate(), info->timestamp()));
// TODO(trevnorris): Setting many object properties in C++ is a significant
// performance hit. Redo this to pass the results to JS and create/set the
// properties there.
#define V(name) \
do { \
obj->Set(name ## _string(), \
Uint32::NewFromUnsigned(isolate(), info->stats()->name())); \
} while (0)
V(total_heap_size);
V(total_heap_size_executable);
V(total_physical_size);
V(used_heap_size);
V(heap_size_limit);
#undef V
}
MakeCallback(this,
Null(isolate()),
gc_info_callback_function(),
ARRAY_SIZE(argv),
argv);
}


void Environment::StartGarbageCollectionTracking(Local<Function> callback) {
ASSERT(gc_info_callback_function().IsEmpty() == true);
set_gc_info_callback_function(callback);
isolate_data()->StartGarbageCollectionTracking(this);
}


void Environment::StopGarbageCollectionTracking() {
ASSERT(gc_info_callback_function().IsEmpty() == false);
isolate_data()->StopGarbageCollectionTracking(this);
set_gc_info_callback_function(Local<Function>());
}


void StartGarbageCollectionTracking(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsFunction() == true);
Environment* env = Environment::GetCurrent(args);
env->StartGarbageCollectionTracking(args[0].As<Function>());
}


void GetHeapStatistics(const FunctionCallbackInfo<Value>& args) {
Expand All @@ -201,11 +63,6 @@ void GetHeapStatistics(const FunctionCallbackInfo<Value>& args) {
}


void StopGarbageCollectionTracking(const FunctionCallbackInfo<Value>& args) {
Environment::GetCurrent(args)->StopGarbageCollectionTracking();
}


void SetFlagsFromString(const FunctionCallbackInfo<Value>& args) {
String::Utf8Value flags(args[0]);
V8::SetFlagsFromString(*flags, flags.length());
Expand All @@ -216,12 +73,6 @@ void InitializeV8Bindings(Handle<Object> target,
Handle<Value> unused,
Handle<Context> context) {
Environment* env = Environment::GetCurrent(context);
env->SetMethod(target,
"startGarbageCollectionTracking",
StartGarbageCollectionTracking);
env->SetMethod(target,
"stopGarbageCollectionTracking",
StopGarbageCollectionTracking);
env->SetMethod(target, "getHeapStatistics", GetHeapStatistics);
env->SetMethod(target, "setFlagsFromString", SetFlagsFromString);
}
Expand Down
Loading

0 comments on commit dab6f68

Please sign in to comment.