Skip to content

Commit

Permalink
process: improve cwd performance
Browse files Browse the repository at this point in the history
This caches the current working directory and only updates the variable
if `process.chdir()` is called.

PR-URL: #27224
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Backport-PR-URL: #27483
  • Loading branch information
BridgeAR authored and targos committed May 5, 2019
1 parent 7bbf951 commit 91b7f5e
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 11 deletions.
3 changes: 2 additions & 1 deletion lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ if (isMainThread) {
const wrapped = mainThreadSetup.wrapProcessMethods(rawMethods);
process.umask = wrapped.umask;
process.chdir = wrapped.chdir;
process.cwd = wrapped.cwd;

// TODO(joyeecheung): deprecate and remove these underscore methods
process._debugProcess = rawMethods._debugProcess;
Expand All @@ -86,11 +87,11 @@ if (isMainThread) {
process.abort = workerThreadSetup.unavailable('process.abort()');
process.chdir = workerThreadSetup.unavailable('process.chdir()');
process.umask = wrapped.umask;
process.cwd = rawMethods.cwd;
}

// Set up methods on the process object for all threads
{
process.cwd = rawMethods.cwd;
process.dlopen = rawMethods.dlopen;
process.uptime = rawMethods.uptime;

Expand Down
20 changes: 19 additions & 1 deletion lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {
getEnvMessagePort
} = internalBinding('worker');

const workerIo = require('internal/worker/io');
const {
messageTypes: {
// Messages that may be received by workers
Expand All @@ -38,7 +39,7 @@ const {
STDIO_WANTS_MORE_DATA,
},
kStdioWantsMoreDataCallback
} = require('internal/worker/io');
} = workerIo;

const {
fatalException: originalFatalException
Expand Down Expand Up @@ -89,6 +90,7 @@ if (process.env.NODE_CHANNEL_FD) {
port.on('message', (message) => {
if (message.type === LOAD_SCRIPT) {
const {
cwdCounter,
filename,
doEval,
workerData,
Expand All @@ -111,6 +113,22 @@ port.on('message', (message) => {
publicWorker.parentPort = publicPort;
publicWorker.workerData = workerData;

// The counter is only passed to the workers created by the main thread, not
// to workers created by other workers.
let cachedCwd = '';
let lastCounter = -1;
const originalCwd = process.cwd;

process.cwd = function() {
const currentCounter = Atomics.load(cwdCounter, 0);
if (currentCounter === lastCounter)
return cachedCwd;
lastCounter = currentCounter;
cachedCwd = originalCwd();
return cachedCwd;
};
workerIo.sharedCwdCounter = cwdCounter;

if (!hasStdin)
process.stdin.push(null);

Expand Down
17 changes: 15 additions & 2 deletions lib/internal/process/main_thread_only.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,15 @@ const { signals } = internalBinding('constants').os;

// The execution of this function itself should not cause any side effects.
function wrapProcessMethods(binding) {
// Cache the working directory to prevent lots of lookups. If the working
// directory is changed by `chdir`, it'll be updated.
let cachedCwd = '';

function chdir(directory) {
validateString(directory, 'directory');
return binding.chdir(directory);
binding.chdir(directory);
// Mark cache that it requires an update.
cachedCwd = '';
}

function umask(mask) {
Expand All @@ -32,9 +38,16 @@ function wrapProcessMethods(binding) {
return binding.umask(mask);
}

function cwd() {
if (cachedCwd === '')
cachedCwd = binding.cwd();
return cachedCwd;
}

return {
chdir,
umask
umask,
cwd
};
}

Expand Down
19 changes: 17 additions & 2 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

/* global SharedArrayBuffer */

const { Object } = primordials;

const EventEmitter = require('events');
Expand All @@ -16,6 +18,7 @@ const {
const { validateString } = require('internal/validators');
const { getOptionValue } = require('internal/options');

const workerIo = require('internal/worker/io');
const {
drainMessagePort,
MessageChannel,
Expand All @@ -26,8 +29,8 @@ const {
kStdioWantsMoreDataCallback,
setupPortReferencing,
ReadableWorkerStdio,
WritableWorkerStdio,
} = require('internal/worker/io');
WritableWorkerStdio
} = workerIo;
const { deserializeError } = require('internal/error-serdes');
const { pathToFileURL } = require('url');

Expand All @@ -50,6 +53,17 @@ const kParentSideStdio = Symbol('kParentSideStdio');
const SHARE_ENV = Symbol.for('nodejs.worker_threads.SHARE_ENV');
const debug = require('internal/util/debuglog').debuglog('worker');

let cwdCounter;

if (isMainThread) {
cwdCounter = new Uint32Array(new SharedArrayBuffer(4));
const originalChdir = process.chdir;
process.chdir = function(path) {
Atomics.add(cwdCounter, 0, 1);
originalChdir(path);
};
}

class Worker extends EventEmitter {
constructor(filename, options = {}) {
super();
Expand Down Expand Up @@ -131,6 +145,7 @@ class Worker extends EventEmitter {
type: messageTypes.LOAD_SCRIPT,
filename,
doEval: !!options.eval,
cwdCounter: cwdCounter || workerIo.sharedCwdCounter,
workerData: options.workerData,
publicPort: port2,
manifestSrc: getOptionValue('--experimental-policy') ?
Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-process-abort.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';

const common = require('../common');
const assert = require('assert');

if (!common.isMainThread)
common.skip('process.abort() is not available in Workers');

// Check that our built-in methods do not have a prototype/constructor behaviour
// if they don't need to. This could be tested for any of our C++ methods.
assert.strictEqual(process.abort.prototype, undefined);
assert.throws(() => new process.abort(), TypeError);
5 changes: 0 additions & 5 deletions test/parallel/test-process-chdir.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,3 @@ const err = {
};
common.expectsError(function() { process.chdir({}); }, err);
common.expectsError(function() { process.chdir(); }, err);

// Check that our built-in methods do not have a prototype/constructor behaviour
// if they don't need to. This could be tested for any of our C++ methods.
assert.strictEqual(process.cwd.prototype, undefined);
assert.throws(() => new process.cwd(), TypeError);
44 changes: 44 additions & 0 deletions test/parallel/test-worker-process-cwd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict';
const assert = require('assert');
const common = require('../common');
const { Worker, isMainThread, parentPort } = require('worker_threads');

// Do not use isMainThread directly, otherwise the test would time out in case
// it's started inside of another worker thread.
if (!process.env.HAS_STARTED_WORKER) {
process.env.HAS_STARTED_WORKER = '1';
if (!isMainThread) {
common.skip('This test can only run as main thread');
}
const w = new Worker(__filename);
process.chdir('..');
w.on('message', common.mustCall((message) => {
assert.strictEqual(message, process.cwd());
process.chdir('..');
w.postMessage(process.cwd());
}));
} else if (!process.env.SECOND_WORKER) {
process.env.SECOND_WORKER = '1';
const firstCwd = process.cwd();
const w = new Worker(__filename);
w.on('message', common.mustCall((message) => {
assert.strictEqual(message, process.cwd());
parentPort.postMessage(firstCwd);
parentPort.onmessage = common.mustCall((obj) => {
const secondCwd = process.cwd();
assert.strictEqual(secondCwd, obj.data);
assert.notStrictEqual(secondCwd, firstCwd);
w.postMessage(secondCwd);
parentPort.unref();
});
}));
} else {
const firstCwd = process.cwd();
parentPort.postMessage(firstCwd);
parentPort.onmessage = common.mustCall((obj) => {
const secondCwd = process.cwd();
assert.strictEqual(secondCwd, obj.data);
assert.notStrictEqual(secondCwd, firstCwd);
process.exit();
});
}

0 comments on commit 91b7f5e

Please sign in to comment.