Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: reduce process.binding() calls #1367

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,8 @@ const Duplex = require('stream').Duplex;
const debug = util.debuglog('tls');
const Timer = process.binding('timer_wrap').Timer;
const tls_wrap = process.binding('tls_wrap');

// constructor for lazy loading
function createTCP() {
var TCP = process.binding('tcp_wrap').TCP;
return new TCP();
}

// constructor for lazy loading
function createPipe() {
var Pipe = process.binding('pipe_wrap').Pipe;
return new Pipe();
}
const TCP = process.binding('tcp_wrap').TCP;
const Pipe = process.binding('pipe_wrap').Pipe;

function onhandshakestart() {
debug('onhandshakestart');
Expand Down Expand Up @@ -284,7 +274,7 @@ TLSSocket.prototype._wrapHandle = function(handle) {

var options = this._tlsOptions;
if (!handle) {
handle = options.pipe ? createPipe() : createTCP();
handle = options.pipe ? new Pipe() : new TCP();
handle.owner = this;
}

Expand Down
76 changes: 16 additions & 60 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,48 +7,18 @@ const dgram = require('dgram');
const assert = require('assert');
const util = require('util');
const debug = util.debuglog('child_process');
const constants = require('constants');

const Process = process.binding('process_wrap').Process;
const WriteWrap = process.binding('stream_wrap').WriteWrap;
const uv = process.binding('uv');

var spawn_sync; // Lazy-loaded process.binding('spawn_sync')
var constants; // Lazy-loaded process.binding('constants')
const spawn_sync = process.binding('spawn_sync');
const Pipe = process.binding('pipe_wrap').Pipe;
const TTY = process.binding('tty_wrap').TTY;
const TCP = process.binding('tcp_wrap').TCP;
const UDP = process.binding('udp_wrap').UDP;

const errnoException = util._errnoException;
var handleWraps = {};

function handleWrapGetter(name, callback) {
var cons;

Object.defineProperty(handleWraps, name, {
get: function() {
if (cons !== undefined) return cons;
return cons = callback();
}
});
}

handleWrapGetter('Pipe', function() {
return process.binding('pipe_wrap').Pipe;
});

handleWrapGetter('TTY', function() {
return process.binding('tty_wrap').TTY;
});

handleWrapGetter('TCP', function() {
return process.binding('tcp_wrap').TCP;
});

handleWrapGetter('UDP', function() {
return process.binding('udp_wrap').UDP;
});

// constructors for lazy loading
function createPipe(ipc) {
return new handleWraps.Pipe(ipc);
}

function createSocket(pipe, readable) {
var s = new net.Socket({ handle: pipe });
Expand Down Expand Up @@ -417,12 +387,12 @@ function setupChannel(target, channel) {
message.type = 'net.Socket';
} else if (handle instanceof net.Server) {
message.type = 'net.Server';
} else if (handle instanceof process.binding('tcp_wrap').TCP ||
handle instanceof process.binding('pipe_wrap').Pipe) {
} else if (handle instanceof TCP ||
handle instanceof Pipe) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fits on a single line now.

message.type = 'net.Native';
} else if (handle instanceof dgram.Socket) {
message.type = 'dgram.Socket';
} else if (handle instanceof process.binding('udp_wrap').UDP) {
} else if (handle instanceof UDP) {
message.type = 'dgram.Native';
} else {
throw new TypeError("This handle type can't be sent");
Expand Down Expand Up @@ -564,7 +534,7 @@ exports.fork = function(modulePath /*, args, options*/) {

exports._forkChild = function(fd) {
// set process.send()
var p = createPipe(true);
var p = new Pipe(true);
p.open(fd);
p.unref();
setupChannel(process, p);
Expand Down Expand Up @@ -852,7 +822,7 @@ function _validateStdio(stdio, sync) {
};

if (!sync)
a.handle = createPipe();
a.handle = new Pipe();

acc.push(a);
} else if (stdio === 'ipc') {
Expand All @@ -865,7 +835,7 @@ function _validateStdio(stdio, sync) {
throw new Error('You cannot use IPC with synchronous forks');
}

ipc = createPipe(true);
ipc = new Pipe(true);
ipcFd = i;

acc.push({
Expand Down Expand Up @@ -989,10 +959,6 @@ function maybeClose(subprocess) {
function ChildProcess() {
EventEmitter.call(this);

// Initialize TCPWrap and PipeWrap
process.binding('tcp_wrap');
process.binding('pipe_wrap');

var self = this;

this._closesNeeded = 1;
Expand Down Expand Up @@ -1072,10 +1038,10 @@ function flushStdio(subprocess) {


function getHandleWrapType(stream) {
if (stream instanceof handleWraps.Pipe) return 'pipe';
if (stream instanceof handleWraps.TTY) return 'tty';
if (stream instanceof handleWraps.TCP) return 'tcp';
if (stream instanceof handleWraps.UDP) return 'udp';
if (stream instanceof Pipe) return 'pipe';
if (stream instanceof TTY) return 'tty';
if (stream instanceof TCP) return 'tcp';
if (stream instanceof UDP) return 'udp';

return false;
}
Expand Down Expand Up @@ -1177,10 +1143,6 @@ ChildProcess.prototype.spawn = function(options) {
ChildProcess.prototype.kill = function(sig) {
var signal;

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

if (sig === 0) {
signal = 0;
} else if (!sig) {
Expand Down Expand Up @@ -1230,9 +1192,6 @@ function lookupSignal(signal) {
if (typeof signal === 'number')
return signal;

if (!constants)
constants = process.binding('constants');

if (!(signal in constants))
throw new Error('Unknown signal: ' + signal);

Expand Down Expand Up @@ -1280,9 +1239,6 @@ function spawnSync(/*file, args, options*/) {
}
}

if (!spawn_sync)
spawn_sync = process.binding('spawn_sync');

var result = spawn_sync.spawn(options);

if (result.output && options.encoding) {
Expand Down
4 changes: 3 additions & 1 deletion lib/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const util = require('util');
const SCHED_NONE = 1;
const SCHED_RR = 2;

const uv = process.binding('uv');

const cluster = new EventEmitter;
module.exports = cluster;
cluster.Worker = Worker;
Expand Down Expand Up @@ -142,7 +144,7 @@ RoundRobinHandle.prototype.add = function(worker, send) {
// Hack: translate 'EADDRINUSE' error string back to numeric error code.
// It works but ideally we'd have some backchannel between the net and
// cluster modules for stuff like this.
var errno = process.binding('uv')['UV_' + err.errno];
var errno = uv['UV_' + err.errno];
send(errno, null);
});
};
Expand Down
1 change: 0 additions & 1 deletion lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ Socket.prototype.bind = function(port /*, address, callback*/) {
if (typeof arguments[arguments.length - 1] === 'function')
self.once('listening', arguments[arguments.length - 1]);

const UDP = process.binding('udp_wrap').UDP;
if (port instanceof UDP) {
replaceHandle(self, port);
startListening(self);
Expand Down
4 changes: 2 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ const util = require('util');
const pathModule = require('path');

const binding = process.binding('fs');
const constants = process.binding('constants');
const constants = require('constants');
const fs = exports;
const Stream = require('stream').Stream;
const EventEmitter = require('events').EventEmitter;
const FSReqWrap = binding.FSReqWrap;
const FSEvent = process.binding('fs_event_wrap').FSEvent;

const Readable = Stream.Readable;
const Writable = Stream.Writable;
Expand Down Expand Up @@ -1201,7 +1202,6 @@ function FSWatcher() {
EventEmitter.call(this);

var self = this;
var FSEvent = process.binding('fs_event_wrap').FSEvent;
this._handle = new FSEvent();
this._handle.owner = this;

Expand Down
32 changes: 10 additions & 22 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ const util = require('util');
const assert = require('assert');
const cares = process.binding('cares_wrap');
const uv = process.binding('uv');
const Pipe = process.binding('pipe_wrap').Pipe;

const TTY = process.binding('tty_wrap');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming suggests it's the TTY constructor but it's really the tty_wrap object (which has the constructor as a property.)

const TCP = process.binding('tcp_wrap').TCP;
const Pipe = process.binding('pipe_wrap').Pipe;
const TCPConnectWrap = process.binding('tcp_wrap').TCPConnectWrap;
const PipeConnectWrap = process.binding('pipe_wrap').PipeConnectWrap;
const ShutdownWrap = process.binding('stream_wrap').ShutdownWrap;
Expand All @@ -21,23 +23,10 @@ const exceptionWithHostPort = util._exceptionWithHostPort;

function noop() {}

// constructor for lazy loading
function createPipe() {
return new Pipe();
}

// constructor for lazy loading
function createTCP() {
var TCP = process.binding('tcp_wrap').TCP;
return new TCP();
}


function createHandle(fd) {
var tty = process.binding('tty_wrap');
var type = tty.guessHandleType(fd);
if (type === 'PIPE') return createPipe();
if (type === 'TCP') return createTCP();
var type = TTY.guessHandleType(fd);
if (type === 'PIPE') return new Pipe();
if (type === 'TCP') return new TCP();
throw new TypeError('Unsupported fd type: ' + type);
}

Expand Down Expand Up @@ -135,6 +124,7 @@ function Socket(options) {
} else if (options.fd !== undefined) {
this._handle = createHandle(options.fd);
this._handle.open(options.fd);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you undo the whitespace change?

if ((options.fd == 1 || options.fd == 2) &&
(this._handle instanceof Pipe) &&
process.platform === 'win32') {
Expand Down Expand Up @@ -873,7 +863,7 @@ Socket.prototype.connect = function(options, cb) {
debug('pipe', pipe, options.path);

if (!this._handle) {
this._handle = pipe ? createPipe() : createTCP();
this._handle = pipe ? new Pipe() : new TCP();
initSocketHandle(this);
}

Expand Down Expand Up @@ -1095,15 +1085,15 @@ var createServerHandle = exports._createServerHandle =
handle.writable = true;
assert(!address && !port);
} else if (port === -1 && addressType === -1) {
handle = createPipe();
handle = new Pipe();
if (process.platform === 'win32') {
var instances = parseInt(process.env.NODE_PENDING_PIPE_INSTANCES);
if (!isNaN(instances)) {
handle.setPendingInstances(instances);
}
}
} else {
handle = createTCP();
handle = new TCP();
isTCP = true;
}

Expand Down Expand Up @@ -1255,8 +1245,6 @@ Server.prototype.listen = function() {
// When the ip is omitted it can be the second argument.
var backlog = toNumber(arguments[1]) || toNumber(arguments[2]);

const TCP = process.binding('tcp_wrap').TCP;

if (arguments.length === 0 || typeof arguments[0] === 'function') {
// Bind to a random port.
listen(self, null, 0, null, backlog);
Expand Down
3 changes: 2 additions & 1 deletion lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const net = require('net');
const url = require('url');
const util = require('util');
const binding = process.binding('crypto');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crypto?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the different naming is possibly to differentiate it from the result of require('crypto'). Plus it's the only binding being loaded in the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stole it from the other modules - I originally had it as cryptoBinding.


// Allow {CLIENT_RENEG_LIMIT} client-initiated session renegotiations
// every {CLIENT_RENEG_WINDOW} seconds. An error event is emitted if more
Expand Down Expand Up @@ -35,7 +36,7 @@ exports.DEFAULT_CIPHERS = [
exports.DEFAULT_ECDH_CURVE = 'prime256v1';

exports.getCiphers = function() {
const names = process.binding('crypto').getSSLCiphers();
const names = binding.getSSLCiphers();
// Drop all-caps names in favor of their lowercase aliases,
var ctx = {};
names.forEach(function(name) {
Expand Down
4 changes: 2 additions & 2 deletions lib/util.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const uv = process.binding('uv');

const formatRegExp = /%[sdj%]/g;
exports.format = function(f) {
if (typeof f !== 'string') {
Expand Down Expand Up @@ -739,9 +741,7 @@ exports.pump = exports.deprecate(function(readStream, writeStream, callback) {
}, 'util.pump(): Use readableStream.pipe() instead');


var uv;
exports._errnoException = function(err, syscall, original) {
if (uv === undefined) uv = process.binding('uv');
var errname = uv.errname(err);
var message = syscall + ' ' + errname;
if (original)
Expand Down