From baed9d514d0c145fdf696324601f67dda89adf3c Mon Sep 17 00:00:00 2001 From: Ryan Date: Fri, 15 May 2009 16:28:10 +0200 Subject: [PATCH] Inform V8 of external allocations. This is sloppy: after each ObjectWrap allocation the user needs to call ObjectWrap::InformV8ofAllocation(). In addition each class deriving from ObjectWrap needs to implement the virtual method size() which should return the size of the derived class. If I was better at C++ I could possibly make this less ugly. For now this is how it is. Memory usage looks much better after this commit. --- configure | 2 +- src/file.cc | 5 ++++- src/file.h | 2 ++ src/http.cc | 22 ++++++++++++++++++---- src/http.h | 2 ++ src/net.cc | 18 ++++++++++++++++-- src/net.h | 6 +++++- src/node.cc | 11 ++++++++++- src/node.h | 5 +++++ src/timer.cc | 3 ++- src/timer.h | 2 ++ test_http.js | 5 ++--- 12 files changed, 69 insertions(+), 14 deletions(-) diff --git a/configure b/configure index 226ac215e55..191e069d3f9 100755 --- a/configure +++ b/configure @@ -95,7 +95,7 @@ uninstall: test: all @for i in test/test*.js; do \\ echo "\$\$i: "; \\ - build/default/node \$\$i && echo PASS || echo FAIL; \\ + build/debug/node \$\$i && echo PASS || echo FAIL; \\ done clean: diff --git a/src/file.cc b/src/file.cc index 00eeea766b1..95c03efef37 100644 --- a/src/file.cc +++ b/src/file.cc @@ -487,7 +487,10 @@ Handle File::New(const Arguments& args) { HandleScope scope; - new File(args.Holder()); + + File *f = new File(args.Holder()); + ObjectWrap::InformV8ofAllocation(f); + return args.This(); } diff --git a/src/file.h b/src/file.h index a71b83d73e3..b42d1ae22e1 100644 --- a/src/file.h +++ b/src/file.h @@ -20,6 +20,8 @@ class File : ObjectWrap { public: static void Initialize (v8::Handle target); + virtual size_t size (void) { return sizeof(File); } + protected: File (v8::Handle handle); ~File (); diff --git a/src/http.cc b/src/http.cc index ae8d9d6e88c..994ddfc7285 100644 --- a/src/http.cc +++ b/src/http.cc @@ -43,7 +43,7 @@ HTTPConnection::Initialize (Handle target) client_constructor_template = Persistent::New(t); client_constructor_template->Inherit(Connection::constructor_template); client_constructor_template->InstanceTemplate()->SetInternalFieldCount(1); - target->Set(String::NewSymbol("Client"), client_constructor_template->GetFunction()); + target->Set(String::NewSymbol("LowLevelClient"), client_constructor_template->GetFunction()); t = FunctionTemplate::New(v8NewServer); server_constructor_template = Persistent::New(t); @@ -57,7 +57,10 @@ Handle HTTPConnection::v8NewClient (const Arguments& args) { HandleScope scope; - new HTTPConnection(args.This(), HTTP_RESPONSE); + + HTTPConnection *connection = new HTTPConnection(args.This(), HTTP_RESPONSE); + ObjectWrap::InformV8ofAllocation(connection); + return args.This(); } @@ -65,7 +68,10 @@ Handle HTTPConnection::v8NewServer (const Arguments& args) { HandleScope scope; - new HTTPConnection(args.This(), HTTP_REQUEST); + + HTTPConnection *connection = new HTTPConnection(args.This(), HTTP_REQUEST); + ObjectWrap::InformV8ofAllocation(connection); + return args.This(); } @@ -281,6 +287,13 @@ HTTPConnection::HTTPConnection (Handle handle, enum http_parser_type typ parser_.data = this; } + +HTTPConnection::~HTTPConnection ( ) +{ + V8::AdjustAmountOfExternalAllocatedMemory(-sizeof(HTTPConnection)); +} + + Persistent HTTPServer::constructor_template; void @@ -312,7 +325,8 @@ HTTPServer::v8New (const Arguments& args) options = Object::New(); } - new HTTPServer(args.This(), protocol_class, options); + HTTPServer *s = new HTTPServer(args.This(), protocol_class, options); + ObjectWrap::InformV8ofAllocation(s); return args.This(); } diff --git a/src/http.h b/src/http.h index f2eeb105d8d..a3c140c78f4 100644 --- a/src/http.h +++ b/src/http.h @@ -20,6 +20,7 @@ protected: HTTPConnection (v8::Handle handle, enum http_parser_type type); + ~HTTPConnection ( ); void OnReceive (const void *buf, size_t len); @@ -47,6 +48,7 @@ public: protected: static v8::Handle v8New (const v8::Arguments& args); + HTTPServer (v8::Handle handle, v8::Handle protocol_class, v8::Handle options) diff --git a/src/net.cc b/src/net.cc index 515bb646a7d..c8e3626224c 100644 --- a/src/net.cc +++ b/src/net.cc @@ -145,7 +145,10 @@ Handle Connection::v8New (const Arguments& args) { HandleScope scope; - new Connection(args.This()); + + Connection *c = new Connection(args.This()); + ObjectWrap::InformV8ofAllocation(c); + return args.This(); } @@ -274,6 +277,16 @@ Connection::v8Send (const Arguments& args) Connection *connection = NODE_UNWRAP(Connection, args.Holder()); if (!connection) return Handle(); + // XXX + // A lot of improvement can be made here. First of all we're allocating + // oi_bufs for every send which is clearly inefficent - it should use a + // memory pool or ring buffer. In either case, v8 needs to be informed + // about our allocations deallocations via + // V8::AdjustAmountOfExternalAllocatedMemory to give the GC hints about + // what we're doing here. Of course, expressing binary data as an array + // of integers is extremely inefficent. This can improved when v8 bug 270 + // (http://code.google.com/p/v8/issues/detail?id=270) has been addressed. + if (args[0]->IsString()) { // utf8 encoding Local s = args[0]->ToString(); @@ -451,7 +464,8 @@ Acceptor::v8New (const Arguments& args) options = Object::New(); } - new Acceptor(args.This(), connection_handler, options); + Acceptor *a = new Acceptor(args.This(), connection_handler, options); + ObjectWrap::InformV8ofAllocation(a); return args.This(); } diff --git a/src/net.h b/src/net.h index 0911d275208..149bae4fe0b 100644 --- a/src/net.h +++ b/src/net.h @@ -13,6 +13,8 @@ class Connection : public ObjectWrap { public: static void Initialize (v8::Handle target); + virtual size_t size (void) { return sizeof(Connection); }; + protected: /* v8 interface */ static v8::Persistent constructor_template; @@ -100,6 +102,8 @@ class Acceptor : public ObjectWrap { public: static void Initialize (v8::Handle target); + virtual size_t size (void) { return sizeof(Acceptor); }; + protected: static v8::Persistent constructor_template; static v8::Handle v8New (const v8::Arguments& args); @@ -109,7 +113,7 @@ protected: Acceptor (v8::Handle handle, v8::Handle connection_handler, v8::Handle options); - virtual ~Acceptor () { Close(); puts("acceptor gc'd!");} + virtual ~Acceptor () { Close(); } v8::Local GetConnectionHandler (void); diff --git a/src/node.cc b/src/node.cc index e9254ed8866..7218547a103 100644 --- a/src/node.cc +++ b/src/node.cc @@ -53,8 +53,10 @@ ObjectWrap::Detach () if (attach_count_ > 0) attach_count_ -= 1; - if(weak_ && attach_count_ == 0) + if(weak_ && attach_count_ == 0) { + V8::AdjustAmountOfExternalAllocatedMemory(-size()); delete this; + } } void* @@ -87,6 +89,12 @@ ObjectWrap::MakeWeak (Persistent _, void *data) delete obj; } +void +ObjectWrap::InformV8ofAllocation (ObjectWrap *obj) +{ + v8::V8::AdjustAmountOfExternalAllocatedMemory(obj->size()); +} + // Extracts a C string from a V8 Utf8Value. const char* ToCString(const v8::String::Utf8Value& value) @@ -244,6 +252,7 @@ main (int argc, char *argv[]) ev_async_init(&eio_watcher, node_eio_cb); eio_init(eio_want_poll, NULL); + V8::Initialize(); V8::SetFlagsFromCommandLine(&argc, argv, true); if(argc < 2) { diff --git a/src/node.h b/src/node.h index 75ebc3ec829..d6db49da12f 100644 --- a/src/node.h +++ b/src/node.h @@ -30,6 +30,11 @@ public: ObjectWrap (v8::Handle handle); virtual ~ObjectWrap ( ); + virtual size_t size (void) = 0; + + /* This must be called after each new ObjectWrap creation! */ + static void InformV8ofAllocation (node::ObjectWrap *obj); + protected: static void* Unwrap (v8::Handle handle); v8::Persistent handle_; diff --git a/src/timer.cc b/src/timer.cc index c1dbbba7c86..a2dfd75e450 100644 --- a/src/timer.cc +++ b/src/timer.cc @@ -80,7 +80,8 @@ Timer::New (const Arguments& args) ev_tstamp after = (double)(args[1]->IntegerValue()) / 1000.0; ev_tstamp repeat = (double)(args[2]->IntegerValue()) / 1000.0; - new Timer(args.Holder(), callback, after, repeat); + Timer *t = new Timer(args.Holder(), callback, after, repeat); + ObjectWrap::InformV8ofAllocation(t); return args.This(); } diff --git a/src/timer.h b/src/timer.h index 9e386a88997..007cdfcb228 100644 --- a/src/timer.h +++ b/src/timer.h @@ -11,6 +11,8 @@ class Timer : ObjectWrap { public: static void Initialize (v8::Handle target); + virtual size_t size (void) { return sizeof(Timer); } + protected: static v8::Persistent constructor_template; diff --git a/test_http.js b/test_http.js index 86598a41376..5f389b06e27 100644 --- a/test_http.js +++ b/test_http.js @@ -1,8 +1,7 @@ -p({hello: "world"}); new node.http.Server(function (msg) { -// setTimeout(function () { + setTimeout(function () { msg.sendHeader(200, [["Content-Type", "text/plain"]]); msg.sendBody("Hello World"); msg.finish(); -// }, 1); + }, 1000); }).listen(8000, "localhost");