This is a big commit that touches just about every file in the src/
directory. The V8 API has changed in significant ways. The most
important changes are:
* Binding functions take a const v8::FunctionCallbackInfo<T>& argument
rather than a const v8::Arguments& argument.
* Binding functions return void rather than v8::Handle<v8::Value>. The
return value is returned with the args.GetReturnValue().Set() family
of functions.
* v8::Persistent<T> no longer derives from v8::Handle<T> and no longer
allows you to directly dereference the object that the persistent
handle points to. This means that the common pattern of caching
oft-used JS values in a persistent handle no longer quite works,
you first need to reconstruct a v8::Local<T> from the persistent
handle with the Local<T>::New(isolate, persistent) factory method.
A handful of (internal) convenience classes and functions have been
added to make dealing with the new API a little easier.
The most visible one is node::Cached<T>, which wraps a v8::Persistent<T>
with some template sugar. It can hold arbitrary types but so far it's
exclusively used for v8::Strings (which was by far the most commonly
cached handle type.)
All compile time warnings about using deprecated APIs have been
suppressed by updating node's API. Though there are still many function
calls that can accept Isolate, and still need to be updated.
node_isolate had to be added as an extern variable in node.h and
node_object_wrap.h
Also a couple small fixes for Error handling.
Before v8 3.16.6 the error stack message was lazily written when it was
needed, which allowed you to change the message after instantiation.
Then the stack would be written with the new message the first time it
was accessed. Though that has changed. Now it creates the stack message
on instantiation. So setting a different message afterwards won't be
displayed.
This is not a complete fix for the problem. Getting error without any
message isn't very useful.
The following test case occasionally triggered an assert because
write_in_progress_ didn't get cleared on error:
$ cat test.js
require('zlib').gunzip('BAM', console.log);
setTimeout(gc, 10);
$ while true; do node --expose-gc test.js || break; done
{ [Error: incorrect header check] errno: -3, code: 'Z_DATA_ERROR' }
Assertion failed: (!write_in_progress_ && "write in progress"),
function Clear, file ../src/node_zlib.cc, line 71.
Abort trap: 6
Steps to avoid that:
* Initialize all primitive member fields in the constructor.
* Clear the write_in_progress_ member field in ZCtx::Error().
* Ref the ZCtx object as soon as write_in_progress_ is set to true.
Before this commit, it could get GC'ed in the time between setting
the field and the call to ctx->Ref().
Fixes#4783.
Inform V8 that the zlib context object is tied to a large off-heap buffer.
This makes the GC run more often (in theory) and improves the accuracy of
--trace_external_memory.
In zlibBuffer(), don't wait for the garbage collector to reclaim the zlib memory
but release it manually. Reduces memory consumption by a factor of 10 or more
with some workloads.
Test case:
function f() {
require('zlib').deflate('xxx', g);
}
function g() {
setTimeout(f, 5);
}
f();
Observe RSS memory usage with and without this commit. After 10,000 iterations,
RSS stabilizes at ~35 MB with this commit. Without, RSS is over 300 MB and keeps
growing.
Cause: whenever the JS object heap hits the high-water mark, the V8 GC sweeps
it clean, then tries to grow it in order to avoid more sweeps in the near
future. Rule of thumb: the bigger the JS heap, the lazier the GC can be.
A side effect of a bigger heap is that objects now live longer. This is harmless
in general but it affects zlib context objects because those are tied to large
buffers that live outside the JS heap, on the order of 16K per context object.
Ergo, don't wait for the GC to reclaim the memory - it may take a long time.
Fixes#4172.
Inform V8 that the zlib context object is tied to a large off-heap buffer.
This makes the GC run more often (in theory) and improves the accuracy of
--trace_external_memory.
In zlibBuffer(), don't wait for the garbage collector to reclaim the zlib memory
but release it manually. Reduces memory consumption by a factor of 10 or more
with some workloads.
Test case:
function f() {
require('zlib').deflate('xxx', g);
}
function g() {
setTimeout(f, 5);
}
f();
Observe RSS memory usage with and without this commit. After 10,000 iterations,
RSS stabilizes at ~35 MB with this commit. Without, RSS is over 300 MB and keeps
growing.
Cause: whenever the JS object heap hits the high-water mark, the V8 GC sweeps
it clean, then tries to grow it in order to avoid more sweeps in the near
future. Rule of thumb: the bigger the JS heap, the lazier the GC can be.
A side effect of a bigger heap is that objects now live longer. This is harmless
in general but it affects zlib context objects because those are tied to large
buffers that live outside the JS heap, on the order of 16K per context object.
Ergo, don't wait for the GC to reclaim the memory - it may take a long time.
Fixes#4172.
Including <zlib.h> may lead to false positives when the user specifies a bad
path in `./configure --shared-zlib --shared-zlib-includes=/path/to/zlib`. If
a zlib.h exists somewhere on the system include path (common on UNIX systems),
the compiler will include that instead, possibly leading to header mismatch
errors that are hard to debug.
It was decided that the performance benefits that isolates offer (faster spin-up
times for worker processes, faster inter-worker communication, possibly a lower
memory footprint) are not actual bottlenecks for most people and do not outweigh
the potential stability issues and intrusive changes to the code base that
first-class support for isolates requires.
Hence, this commit backs out all isolates-related changes.
Good bye, isolates. We hardly knew ye.
* Add assert to prevent parallel writes
* Embed request object instead of using new/delete
* Remove unnecessary WorkReqWrap in favor of uv_work_t
* Use container_of instead of req->data
Along with 2d8af39acc and
0ad2717fd8, this should Fix#2504.
Ben Noordhuis
Brian White
Trevor Norris
isaacs
ssuda
Fedor Indutny
Bert Belder
Ryan Dahl