Skip to content

Commit

Permalink
Revert support for isolates.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@bnoordhuis
bnoordhuis committed Feb 6, 2012
1 parent a9723df commit 74a8215
Show file tree
Hide file tree
Showing 45 changed files with 413 additions and 2,257 deletions.
6 changes: 0 additions & 6 deletions configure
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ parser.add_option("--without-waf",
dest="without_waf",
help="Don\'t install node-waf")

parser.add_option("--without-isolates",
action="store_true",
dest="without_isolates",
help="Build without isolates (no threads, single loop) [Default: False]")

parser.add_option("--without-ssl",
action="store_true",
dest="without_ssl",
Expand Down Expand Up @@ -172,7 +167,6 @@ def target_arch():

def configure_node(o):
# TODO add gdb
o['variables']['node_use_isolates'] = b(not options.without_isolates)
o['variables']['node_prefix'] = options.prefix if options.prefix else ''
o['variables']['node_use_dtrace'] = b(options.with_dtrace)
o['variables']['node_install_npm'] = b(not options.without_npm)
Expand Down
64 changes: 14 additions & 50 deletions lib/_debugger.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var util = require('util'),
vm = require('vm'),
repl = require('repl'),
inherits = util.inherits,
fork = require('child_process').fork;
spawn = require('child_process').spawn;

exports.start = function(argv, stdin, stdout) {
argv || (argv = process.argv.slice(2));
Expand All @@ -39,7 +39,7 @@ exports.start = function(argv, stdin, stdout) {
stdin = stdin || process.openStdin();
stdout = stdout || process.stdout;

var args = argv,
var args = ['--debug-brk'].concat(argv),
interface = new Interface(stdin, stdout, args);

stdin.resume();
Expand Down Expand Up @@ -169,8 +169,6 @@ function Client() {
this.scripts = {};
this.breakpoints = [];

this.isolates = process.features.isolates;

// Note that 'Protocol' requires strings instead of Buffers.
socket.setEncoding('utf8');
socket.on('data', function(d) {
Expand Down Expand Up @@ -1597,51 +1595,20 @@ Interface.prototype.trySpawn = function(cb) {
}
}

var client = self.client = new Client(),
connectionAttempts = 0;

if (!this.child) {
if (client.isolates) {
this.child = fork(this.args.shift(), this.args, {
thread: true,
debug: function(d) {
d.onmessage = function(event) {
client._onResponse({
headers: {},
body: JSON.parse(event)
});
};

// Monkey patch client to send requests directly to debugger
client.req = function(req, cb) {
req.type = 'request';
cb.request_seq = req.seq = this.protocol.reqSeq++;
this._reqCallbacks.push(cb);

d.write(JSON.stringify(req));
};

client.emit('ready');

client._onResponse({
headers: { Type: 'connect' },
body: {}
});
},
debugBrk: true
});
this.child.kill = function() {
self.error('isolate.kill is not implemented yet!');
};
} else {
this.child = fork('--debug-brk', this.args);
}
this.child = spawn(process.execPath, this.args);

this.child.stdout.on('data', this.childPrint.bind(this));
this.child.stderr.on('data', this.childPrint.bind(this));
}

this.pause();

var client = self.client = new Client(),
connectionAttempts = 0;

client.once('ready', function() {
if (!client.isolates) self.stdout.write(' ok\n');
self.stdout.write(' ok\n');

// Restore breakpoints
breakpoints.forEach(function(bp) {
Expand Down Expand Up @@ -1689,14 +1656,11 @@ Interface.prototype.trySpawn = function(cb) {
function attemptConnect() {
++connectionAttempts;
self.stdout.write('.');

client.connect(port, host);
}

if (!client.isolates) {
setTimeout(function() {
self.print('connecting..', true);
attemptConnect();
}, 50);
}
setTimeout(function() {
self.print('connecting..', true);
attemptConnect();
}, 50);
};
69 changes: 4 additions & 65 deletions lib/child_process.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,11 @@ exports.fork = function(modulePath /*, args, options*/) {
options.env.NODE_CHANNEL_FD = 42;

// stdin is the IPC channel.
if (!options.thread) options.stdinStream = createPipe(true);
options.stdinStream = createPipe(true);

var child = spawn(process.execPath, args, options);

if (!options.thread) setupChannel(child, options.stdinStream);
setupChannel(child, options.stdinStream);

return child;
};
Expand Down Expand Up @@ -370,7 +370,7 @@ var spawn = exports.spawn = function(file, args, options) {
envPairs.push(key + '=' + env[key]);
}

var child = (options && options.thread) ? (new Isolate) : (new ChildProcess);
var child = new ChildProcess();

child.spawn({
file: file,
Expand All @@ -379,8 +379,7 @@ var spawn = exports.spawn = function(file, args, options) {
windowsVerbatimArguments: !!(options && options.windowsVerbatimArguments),
envPairs: envPairs,
customFds: options ? options.customFds : null,
stdinStream: options ? options.stdinStream : null,
options: options
stdinStream: options ? options.stdinStream : null
});

return child;
Expand Down Expand Up @@ -537,63 +536,3 @@ ChildProcess.prototype.kill = function(sig) {
// TODO: raise error if r == -1?
}
};


// Lazy loaded.
var isolates = null;


function Isolate() {
if (!process.features.isolates) {
throw new Error('Compiled without isolates support.');
}

if (!isolates) {
isolates = process.binding('isolates');
}

this._handle = null;
}
inherits(Isolate, EventEmitter); // maybe inherit from ChildProcess?


Isolate.prototype.spawn = function(options) {
var self = this;

if (self._handle) throw new Error('Isolate already running.');
self._handle = isolates.create(options.args, options.options);
if (!self._handle) throw new Error('Cannot create isolate.');

self._handle.onmessage = function(msg, recvHandle) {
msg = JSON.parse('' + msg);

// Update simultaneous accepts on Windows
net._setSimultaneousAccepts(recvHandle);

self.emit('message', msg, recvHandle);
};

self._handle.onexit = function() {
self._handle = null;
self.emit('exit');
};
};


Isolate.prototype.kill = function(sig) {
if (!this._handle) throw new Error('Isolate not running.');
// ignore silently for now, need a way to signal the other thread
};


Isolate.prototype.send = function(msg, sendHandle) {
if (typeof msg === 'undefined') throw new TypeError('Bad argument.');
if (!this._handle) throw new Error('Isolate not running.');
msg = JSON.stringify(msg);
msg = new Buffer(msg);

// Update simultaneous accepts on Windows
net._setSimultaneousAccepts(sendHandle);

return this._handle.send(msg, sendHandle);
};
12 changes: 0 additions & 12 deletions node.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
'node_use_dtrace': 'false',
'node_use_openssl%': 'true',
'node_use_system_openssl%': 'false',
'node_use_isolates%': 'true',
'library_files': [
'src/node.js',
'lib/_debugger.js',
Expand Down Expand Up @@ -73,9 +72,7 @@
'src/cares_wrap.cc',
'src/handle_wrap.cc',
'src/node.cc',
'src/node_vars.cc',
'src/node_buffer.cc',
'src/node_isolate.cc',
'src/node_constants.cc',
'src/node_extensions.cc',
'src/node_file.cc',
Expand All @@ -95,12 +92,9 @@
'src/v8_typed_array.cc',
'src/udp_wrap.cc',
# headers to make for a more pleasant IDE experience
'src/ngx-queue.h',
'src/handle_wrap.h',
'src/node.h',
'src/node_vars.h',
'src/node_buffer.h',
'src/node_isolate.h',
'src/node_constants.h',
'src/node_crypto.h',
'src/node_extensions.h',
Expand Down Expand Up @@ -133,12 +127,6 @@
],

'conditions': [
[ 'node_use_isolates=="true"', {
'defines': [ 'HAVE_ISOLATES=1' ],
}, {
'defines': [ 'HAVE_ISOLATES=0' ],
}],

[ 'node_use_openssl=="true"', {
'defines': [ 'HAVE_OPENSSL=1' ],
'sources': [ 'src/node_crypto.cc' ],
Expand Down
19 changes: 9 additions & 10 deletions src/cares_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <assert.h>
#include <node.h>
#include <req_wrap.h>
#include <node_vars.h>
#include <uv.h>

#include <string.h>
Expand All @@ -49,11 +48,6 @@
#endif


#include <node_vars.h>
#define oncomplete_sym NODE_VAR(oncomplete_sym)
#define ares_channel NODE_VAR(ares_channel)


namespace node {

namespace cares_wrap {
Expand All @@ -75,6 +69,11 @@ using v8::Value;

typedef class ReqWrap<uv_getaddrinfo_t> GetAddrInfoReqWrap;

static Persistent<String> oncomplete_sym;

static ares_channel ares_channel;


static Local<Array> HostentToAddresses(struct hostent* host) {
HandleScope scope;
Local<Array> addresses = Array::New();
Expand Down Expand Up @@ -608,7 +607,7 @@ void AfterGetAddrInfo(uv_getaddrinfo_t* req, int status, struct addrinfo* res) {

if (status) {
// Error
SetErrno(uv_last_error(Loop()));
SetErrno(uv_last_error(uv_default_loop()));
argv[0] = Local<Value>::New(Null());
} else {
// Success
Expand Down Expand Up @@ -711,7 +710,7 @@ static Handle<Value> GetAddrInfo(const Arguments& args) {
hints.ai_family = fam;
hints.ai_socktype = SOCK_STREAM;

int r = uv_getaddrinfo(Loop(),
int r = uv_getaddrinfo(uv_default_loop(),
&req_wrap->req_,
AfterGetAddrInfo,
*hostname,
Expand All @@ -720,7 +719,7 @@ static Handle<Value> GetAddrInfo(const Arguments& args) {
req_wrap->Dispatched();

if (r) {
SetErrno(uv_last_error(Loop()));
SetErrno(uv_last_error(uv_default_loop()));
delete req_wrap;
return scope.Close(v8::Null());
} else {
Expand All @@ -737,7 +736,7 @@ static void Initialize(Handle<Object> target) {
assert(r == ARES_SUCCESS);

struct ares_options options;
uv_ares_init_options(Loop(), &ares_channel, &options, 0);
uv_ares_init_options(uv_default_loop(), &ares_channel, &options, 0);
assert(r == 0);

NODE_SET_METHOD(target, "queryA", Query<QueryAWrap>);
Expand Down
9 changes: 4 additions & 5 deletions src/fs_event_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

#include <node.h>
#include <handle_wrap.h>
#include <node_vars.h>

#include <stdlib.h>

Expand Down Expand Up @@ -110,15 +109,15 @@ Handle<Value> FSEventWrap::Start(const Arguments& args) {

String::Utf8Value path(args[0]->ToString());

int r = uv_fs_event_init(Loop(), &wrap->handle_, *path, OnEvent, 0);
int r = uv_fs_event_init(uv_default_loop(), &wrap->handle_, *path, OnEvent, 0);
if (r == 0) {
// Check for persistent argument
if (!args[1]->IsTrue()) {
uv_unref(Loop());
uv_unref(uv_default_loop());
}
wrap->initialized_ = true;
} else {
SetErrno(uv_last_error(Loop()));
SetErrno(uv_last_error(uv_default_loop()));
}

return scope.Close(Integer::New(r));
Expand Down Expand Up @@ -146,7 +145,7 @@ void FSEventWrap::OnEvent(uv_fs_event_t* handle, const char* filename,
// assumption that a rename implicitly means an attribute change. Not too
// unreasonable, right? Still, we should revisit this before v1.0.
if (status) {
SetErrno(uv_last_error(Loop()));
SetErrno(uv_last_error(uv_default_loop()));
eventStr = String::Empty();
}
else if (events & UV_RENAME) {
Expand Down
5 changes: 2 additions & 3 deletions src/handle_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

#include <node.h>
#include <handle_wrap.h>
#include <node_vars.h>

namespace node {

Expand Down Expand Up @@ -71,7 +70,7 @@ Handle<Value> HandleWrap::Unref(const Arguments& args) {
}

wrap->unref = true;
uv_unref(Loop());
uv_unref(uv_default_loop());

return v8::Undefined();
}
Expand All @@ -89,7 +88,7 @@ Handle<Value> HandleWrap::Ref(const Arguments& args) {
}

wrap->unref = false;
uv_ref(Loop());
uv_ref(uv_default_loop());

return v8::Undefined();
}
Expand Down
Loading

0 comments on commit 74a8215

Please sign in to comment.