From eb6ec94a39a5fa2f9c28029540c9f5032f09494c Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 14 Apr 2019 17:34:08 +0200 Subject: [PATCH 1/3] process: improve cwd performance This caches the current working directory and only updates the variable if `process.chdir()` is called. --- lib/internal/bootstrap/node.js | 3 +- lib/internal/main/worker_thread.js | 20 ++++++++++++- lib/internal/process/main_thread_only.js | 17 +++++++++-- lib/internal/worker.js | 19 ++++++++++-- test/parallel/test-worker-process-cwd.js | 38 ++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-worker-process-cwd.js diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index a9fff289371e91..ddf1b9825704ee 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -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; @@ -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; diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 46152f6742e5d3..932f282a1bae2c 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -25,6 +25,7 @@ const { getEnvMessagePort } = internalBinding('worker'); +const workerIo = require('internal/worker/io'); const { messageTypes: { // Messages that may be received by workers @@ -38,7 +39,7 @@ const { STDIO_WANTS_MORE_DATA, }, kStdioWantsMoreDataCallback -} = require('internal/worker/io'); +} = workerIo; const { fatalException: originalFatalException @@ -89,6 +90,7 @@ if (process.env.NODE_CHANNEL_FD) { port.on('message', (message) => { if (message.type === LOAD_SCRIPT) { const { + cwdCounter, filename, doEval, workerData, @@ -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); diff --git a/lib/internal/process/main_thread_only.js b/lib/internal/process/main_thread_only.js index 358e2b37b0f834..0cb3edbf9ad7b4 100644 --- a/lib/internal/process/main_thread_only.js +++ b/lib/internal/process/main_thread_only.js @@ -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) { @@ -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 }; } diff --git a/lib/internal/worker.js b/lib/internal/worker.js index e9d961f35a0a71..fc2a5eceeddf8e 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -1,5 +1,7 @@ 'use strict'; +/* global SharedArrayBuffer */ + const { Object } = primordials; const EventEmitter = require('events'); @@ -16,6 +18,7 @@ const { const { validateString } = require('internal/validators'); const { getOptionValue } = require('internal/options'); +const workerIo = require('internal/worker/io'); const { drainMessagePort, MessageChannel, @@ -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'); @@ -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(); @@ -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') ? diff --git a/test/parallel/test-worker-process-cwd.js b/test/parallel/test-worker-process-cwd.js new file mode 100644 index 00000000000000..ec4a37a6604f57 --- /dev/null +++ b/test/parallel/test-worker-process-cwd.js @@ -0,0 +1,38 @@ +'use strict'; +const assert = require('assert'); +const common = require('../common'); +const { Worker, isMainThread, parentPort } = require('worker_threads'); + +if (isMainThread) { + 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(); + }); +} From 27a2b06b613c6e466a0ed1285fc70317e4aba6cc Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 23 Apr 2019 16:11:28 +0200 Subject: [PATCH 2/3] fixup! process: improve cwd performance --- test/parallel/test-process-abort.js | 12 ++++++++++++ test/parallel/test-process-chdir.js | 5 ----- 2 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-process-abort.js diff --git a/test/parallel/test-process-abort.js b/test/parallel/test-process-abort.js new file mode 100644 index 00000000000000..665e1399a3f362 --- /dev/null +++ b/test/parallel/test-process-abort.js @@ -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); diff --git a/test/parallel/test-process-chdir.js b/test/parallel/test-process-chdir.js index 005e17fac25a90..e66d366fb7874f 100644 --- a/test/parallel/test-process-chdir.js +++ b/test/parallel/test-process-chdir.js @@ -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); From 9942bf409a7528360dc5a6a6c7fdb8234117a43d Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 25 Apr 2019 05:13:53 +0200 Subject: [PATCH 3/3] fixup! process: improve cwd performance --- test/parallel/test-worker-process-cwd.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-worker-process-cwd.js b/test/parallel/test-worker-process-cwd.js index ec4a37a6604f57..dec70ac07c8f11 100644 --- a/test/parallel/test-worker-process-cwd.js +++ b/test/parallel/test-worker-process-cwd.js @@ -3,7 +3,13 @@ const assert = require('assert'); const common = require('../common'); const { Worker, isMainThread, parentPort } = require('worker_threads'); -if (isMainThread) { +// 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) => {