Skip to content

Commit 74a8215

Browse files
bnoordhuisbnoordhuis
committed
Revert support for isolates.
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.
1 parent a9723df commit 74a8215

45 files changed

Lines changed: 413 additions & 2257 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎configure‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,6 @@ parser.add_option("--without-waf",
3232
dest="without_waf",
3333
help="Don\'t install node-waf")
3434

35-
parser.add_option("--without-isolates",
36-
action="store_true",
37-
dest="without_isolates",
38-
help="Build without isolates (no threads, single loop) [Default: False]")
39-
4035
parser.add_option("--without-ssl",
4136
action="store_true",
4237
dest="without_ssl",
@@ -172,7 +167,6 @@ def target_arch():
172167

173168
def configure_node(o):
174169
# TODO add gdb
175-
o['variables']['node_use_isolates'] = b(not options.without_isolates)
176170
o['variables']['node_prefix'] = options.prefix if options.prefix else ''
177171
o['variables']['node_use_dtrace'] = b(options.with_dtrace)
178172
o['variables']['node_install_npm'] = b(not options.without_npm)

‎lib/_debugger.js‎

Lines changed: 14 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ var util = require('util'),
2525
vm = require('vm'),
2626
repl = require('repl'),
2727
inherits = util.inherits,
28-
fork = require('child_process').fork;
28+
spawn = require('child_process').spawn;
2929

3030
exports.start = function(argv, stdin, stdout) {
3131
argv || (argv = process.argv.slice(2));
@@ -39,7 +39,7 @@ exports.start = function(argv, stdin, stdout) {
3939
stdin = stdin || process.openStdin();
4040
stdout = stdout || process.stdout;
4141

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

4545
stdin.resume();
@@ -169,8 +169,6 @@ function Client() {
169169
this.scripts = {};
170170
this.breakpoints = [];
171171

172-
this.isolates = process.features.isolates;
173-
174172
// Note that 'Protocol' requires strings instead of Buffers.
175173
socket.setEncoding('utf8');
176174
socket.on('data', function(d) {
@@ -1597,51 +1595,20 @@ Interface.prototype.trySpawn = function(cb) {
15971595
}
15981596
}
15991597

1600-
var client = self.client = new Client(),
1601-
connectionAttempts = 0;
1602-
16031598
if (!this.child) {
1604-
if (client.isolates) {
1605-
this.child = fork(this.args.shift(), this.args, {
1606-
thread: true,
1607-
debug: function(d) {
1608-
d.onmessage = function(event) {
1609-
client._onResponse({
1610-
headers: {},
1611-
body: JSON.parse(event)
1612-
});
1613-
};
1614-
1615-
// Monkey patch client to send requests directly to debugger
1616-
client.req = function(req, cb) {
1617-
req.type = 'request';
1618-
cb.request_seq = req.seq = this.protocol.reqSeq++;
1619-
this._reqCallbacks.push(cb);
1620-
1621-
d.write(JSON.stringify(req));
1622-
};
1623-
1624-
client.emit('ready');
1625-
1626-
client._onResponse({
1627-
headers: { Type: 'connect' },
1628-
body: {}
1629-
});
1630-
},
1631-
debugBrk: true
1632-
});
1633-
this.child.kill = function() {
1634-
self.error('isolate.kill is not implemented yet!');
1635-
};
1636-
} else {
1637-
this.child = fork('--debug-brk', this.args);
1638-
}
1599+
this.child = spawn(process.execPath, this.args);
1600+
1601+
this.child.stdout.on('data', this.childPrint.bind(this));
1602+
this.child.stderr.on('data', this.childPrint.bind(this));
16391603
}
16401604

16411605
this.pause();
16421606

1607+
var client = self.client = new Client(),
1608+
connectionAttempts = 0;
1609+
16431610
client.once('ready', function() {
1644-
if (!client.isolates) self.stdout.write(' ok\n');
1611+
self.stdout.write(' ok\n');
16451612

16461613
// Restore breakpoints
16471614
breakpoints.forEach(function(bp) {
@@ -1689,14 +1656,11 @@ Interface.prototype.trySpawn = function(cb) {
16891656
function attemptConnect() {
16901657
++connectionAttempts;
16911658
self.stdout.write('.');
1692-
16931659
client.connect(port, host);
16941660
}
16951661

1696-
if (!client.isolates) {
1697-
setTimeout(function() {
1698-
self.print('connecting..', true);
1699-
attemptConnect();
1700-
}, 50);
1701-
}
1662+
setTimeout(function() {
1663+
self.print('connecting..', true);
1664+
attemptConnect();
1665+
}, 50);
17021666
};

‎lib/child_process.js‎

Lines changed: 4 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,11 @@ exports.fork = function(modulePath /*, args, options*/) {
209209
options.env.NODE_CHANNEL_FD = 42;
210210

211211
// stdin is the IPC channel.
212-
if (!options.thread) options.stdinStream = createPipe(true);
212+
options.stdinStream = createPipe(true);
213213

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

216-
if (!options.thread) setupChannel(child, options.stdinStream);
216+
setupChannel(child, options.stdinStream);
217217

218218
return child;
219219
};
@@ -370,7 +370,7 @@ var spawn = exports.spawn = function(file, args, options) {
370370
envPairs.push(key + '=' + env[key]);
371371
}
372372

373-
var child = (options && options.thread) ? (new Isolate) : (new ChildProcess);
373+
var child = new ChildProcess();
374374

375375
child.spawn({
376376
file: file,
@@ -379,8 +379,7 @@ var spawn = exports.spawn = function(file, args, options) {
379379
windowsVerbatimArguments: !!(options && options.windowsVerbatimArguments),
380380
envPairs: envPairs,
381381
customFds: options ? options.customFds : null,
382-
stdinStream: options ? options.stdinStream : null,
383-
options: options
382+
stdinStream: options ? options.stdinStream : null
384383
});
385384

386385
return child;
@@ -537,63 +536,3 @@ ChildProcess.prototype.kill = function(sig) {
537536
// TODO: raise error if r == -1?
538537
}
539538
};
540-
541-
542-
// Lazy loaded.
543-
var isolates = null;
544-
545-
546-
function Isolate() {
547-
if (!process.features.isolates) {
548-
throw new Error('Compiled without isolates support.');
549-
}
550-
551-
if (!isolates) {
552-
isolates = process.binding('isolates');
553-
}
554-
555-
this._handle = null;
556-
}
557-
inherits(Isolate, EventEmitter); // maybe inherit from ChildProcess?
558-
559-
560-
Isolate.prototype.spawn = function(options) {
561-
var self = this;
562-
563-
if (self._handle) throw new Error('Isolate already running.');
564-
self._handle = isolates.create(options.args, options.options);
565-
if (!self._handle) throw new Error('Cannot create isolate.');
566-
567-
self._handle.onmessage = function(msg, recvHandle) {
568-
msg = JSON.parse('' + msg);
569-
570-
// Update simultaneous accepts on Windows
571-
net._setSimultaneousAccepts(recvHandle);
572-
573-
self.emit('message', msg, recvHandle);
574-
};
575-
576-
self._handle.onexit = function() {
577-
self._handle = null;
578-
self.emit('exit');
579-
};
580-
};
581-
582-
583-
Isolate.prototype.kill = function(sig) {
584-
if (!this._handle) throw new Error('Isolate not running.');
585-
// ignore silently for now, need a way to signal the other thread
586-
};
587-
588-
589-
Isolate.prototype.send = function(msg, sendHandle) {
590-
if (typeof msg === 'undefined') throw new TypeError('Bad argument.');
591-
if (!this._handle) throw new Error('Isolate not running.');
592-
msg = JSON.stringify(msg);
593-
msg = new Buffer(msg);
594-
595-
// Update simultaneous accepts on Windows
596-
net._setSimultaneousAccepts(sendHandle);
597-
598-
return this._handle.send(msg, sendHandle);
599-
};

‎node.gyp‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
'node_use_dtrace': 'false',
99
'node_use_openssl%': 'true',
1010
'node_use_system_openssl%': 'false',
11-
'node_use_isolates%': 'true',
1211
'library_files': [
1312
'src/node.js',
1413
'lib/_debugger.js',
@@ -73,9 +72,7 @@
7372
'src/cares_wrap.cc',
7473
'src/handle_wrap.cc',
7574
'src/node.cc',
76-
'src/node_vars.cc',
7775
'src/node_buffer.cc',
78-
'src/node_isolate.cc',
7976
'src/node_constants.cc',
8077
'src/node_extensions.cc',
8178
'src/node_file.cc',
@@ -95,12 +92,9 @@
9592
'src/v8_typed_array.cc',
9693
'src/udp_wrap.cc',
9794
# headers to make for a more pleasant IDE experience
98-
'src/ngx-queue.h',
9995
'src/handle_wrap.h',
10096
'src/node.h',
101-
'src/node_vars.h',
10297
'src/node_buffer.h',
103-
'src/node_isolate.h',
10498
'src/node_constants.h',
10599
'src/node_crypto.h',
106100
'src/node_extensions.h',
@@ -133,12 +127,6 @@
133127
],
134128

135129
'conditions': [
136-
[ 'node_use_isolates=="true"', {
137-
'defines': [ 'HAVE_ISOLATES=1' ],
138-
}, {
139-
'defines': [ 'HAVE_ISOLATES=0' ],
140-
}],
141-
142130
[ 'node_use_openssl=="true"', {
143131
'defines': [ 'HAVE_OPENSSL=1' ],
144132
'sources': [ 'src/node_crypto.cc' ],

‎src/cares_wrap.cc‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include <assert.h>
2323
#include <node.h>
2424
#include <req_wrap.h>
25-
#include <node_vars.h>
2625
#include <uv.h>
2726

2827
#include <string.h>
@@ -49,11 +48,6 @@
4948
#endif
5049

5150

52-
#include <node_vars.h>
53-
#define oncomplete_sym NODE_VAR(oncomplete_sym)
54-
#define ares_channel NODE_VAR(ares_channel)
55-
56-
5751
namespace node {
5852

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

7670
typedef class ReqWrap<uv_getaddrinfo_t> GetAddrInfoReqWrap;
7771

72+
static Persistent<String> oncomplete_sym;
73+
74+
static ares_channel ares_channel;
75+
76+
7877
static Local<Array> HostentToAddresses(struct hostent* host) {
7978
HandleScope scope;
8079
Local<Array> addresses = Array::New();
@@ -608,7 +607,7 @@ void AfterGetAddrInfo(uv_getaddrinfo_t* req, int status, struct addrinfo* res) {
608607

609608
if (status) {
610609
// Error
611-
SetErrno(uv_last_error(Loop()));
610+
SetErrno(uv_last_error(uv_default_loop()));
612611
argv[0] = Local<Value>::New(Null());
613612
} else {
614613
// Success
@@ -711,7 +710,7 @@ static Handle<Value> GetAddrInfo(const Arguments& args) {
711710
hints.ai_family = fam;
712711
hints.ai_socktype = SOCK_STREAM;
713712

714-
int r = uv_getaddrinfo(Loop(),
713+
int r = uv_getaddrinfo(uv_default_loop(),
715714
&req_wrap->req_,
716715
AfterGetAddrInfo,
717716
*hostname,
@@ -720,7 +719,7 @@ static Handle<Value> GetAddrInfo(const Arguments& args) {
720719
req_wrap->Dispatched();
721720

722721
if (r) {
723-
SetErrno(uv_last_error(Loop()));
722+
SetErrno(uv_last_error(uv_default_loop()));
724723
delete req_wrap;
725724
return scope.Close(v8::Null());
726725
} else {
@@ -737,7 +736,7 @@ static void Initialize(Handle<Object> target) {
737736
assert(r == ARES_SUCCESS);
738737

739738
struct ares_options options;
740-
uv_ares_init_options(Loop(), &ares_channel, &options, 0);
739+
uv_ares_init_options(uv_default_loop(), &ares_channel, &options, 0);
741740
assert(r == 0);
742741

743742
NODE_SET_METHOD(target, "queryA", Query<QueryAWrap>);

‎src/fs_event_wrap.cc‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
#include <node.h>
2323
#include <handle_wrap.h>
24-
#include <node_vars.h>
2524

2625
#include <stdlib.h>
2726

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

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

113-
int r = uv_fs_event_init(Loop(), &wrap->handle_, *path, OnEvent, 0);
112+
int r = uv_fs_event_init(uv_default_loop(), &wrap->handle_, *path, OnEvent, 0);
114113
if (r == 0) {
115114
// Check for persistent argument
116115
if (!args[1]->IsTrue()) {
117-
uv_unref(Loop());
116+
uv_unref(uv_default_loop());
118117
}
119118
wrap->initialized_ = true;
120119
} else {
121-
SetErrno(uv_last_error(Loop()));
120+
SetErrno(uv_last_error(uv_default_loop()));
122121
}
123122

124123
return scope.Close(Integer::New(r));
@@ -146,7 +145,7 @@ void FSEventWrap::OnEvent(uv_fs_event_t* handle, const char* filename,
146145
// assumption that a rename implicitly means an attribute change. Not too
147146
// unreasonable, right? Still, we should revisit this before v1.0.
148147
if (status) {
149-
SetErrno(uv_last_error(Loop()));
148+
SetErrno(uv_last_error(uv_default_loop()));
150149
eventStr = String::Empty();
151150
}
152151
else if (events & UV_RENAME) {

‎src/handle_wrap.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
#include <node.h>
2323
#include <handle_wrap.h>
24-
#include <node_vars.h>
2524

2625
namespace node {
2726

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

7372
wrap->unref = true;
74-
uv_unref(Loop());
73+
uv_unref(uv_default_loop());
7574

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

9190
wrap->unref = false;
92-
uv_ref(Loop());
91+
uv_ref(uv_default_loop());
9392

9493
return v8::Undefined();
9594
}

0 commit comments

Comments
 (0)