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

process: improve cwd performance #27224

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
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;

BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
// 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;

BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
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.
BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
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();
});
}