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

re-enable the "worker_threads" sync workaround #1021

Merged
merged 2 commits into from
Mar 20, 2021
Merged
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
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ jobs:
- name: Register Test
run: node scripts/register-test.js

- name: Register Test (ESBUILD_WORKER_THREADS)
if: matrix.os != 'windows-latest'
run: ESBUILD_WORKER_THREADS=1 node scripts/register-test.js

- name: Verify Source Map
run: node scripts/verify-source-map.js

Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@

This was tripping up esbuild's TypeScript parser because the `>=` token was split into a `>` token and a `=` token because the `>` token is needed to close the type parameter list, but the `=` token was not being combined with the following `>` token to form a `=>` token. This is normally not an issue because there is normally a space in between the `>` and the `=>` tokens here. The issue only happened when the spaces were removed. This bug has been fixed. Now after the `>=` token is split, esbuild will expand the `=` token into the following characters if possible, which can result in a `=>`, `==`, or `===` token.

* Enable faster synchronous transforms under a flag ([#1000](https://github.com/evanw/esbuild/issues/1000))

Currently the synchronous JavaScript API calls `transformSync` and `buildSync` spawn a new child process on every call. This is due to limitations with node's `child_process` API. Doing this means `transformSync` and `buildSync` are much slower than `transform` and `build`, which share the same child process across calls.

There was previously a workaround for this limitation that uses node's `worker_threads` API and atomics to block the main thread while asynchronous communication happens in a worker, but that was reverted due to a bug in node's `worker_threads` implementation. Now that this bug has been fixed by node, I am re-enabling this workaround. This should result in `transformSync` and `buildSync` being much faster.

This approach is experimental and is currently only enabled if the `ESBUILD_WORKER_THREADS` environment variable is present. If this use case matters to you, please try it out and let me know if you find any problems with it.

## 0.9.5

* Fix parsing of the `[dir]` placeholder ([#1013](https://github.com/evanw/esbuild/issues/1013))
Expand Down
194 changes: 192 additions & 2 deletions lib/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,21 @@ declare const ESBUILD_VERSION: string;
// package. "WASM" will be true for "esbuild-wasm" and false for "esbuild".
declare const WASM: boolean;

let worker_threads: typeof import('worker_threads') | undefined;

// This optimization is opt-in for now because it could break if node has bugs
// with "worker_threads", and node has had such bugs in the past.
//
// TODO: Determine under which conditions this is safe to enable, and then
// replace this check with a check for those conditions.
if (process.env.ESBUILD_WORKER_THREADS) {
// Don't crash if the "worker_threads" library isn't present
try {
worker_threads = require('worker_threads');
} catch {
}
}

let esbuildCommandAndArgs = (): [string, string[]] => {
// This feature was added to give external code a way to modify the binary
// path without modifying the code itself. Do not remove this because
Expand Down Expand Up @@ -70,15 +85,27 @@ export let transform: typeof types.transform = (input, options) =>
ensureServiceIsRunning().transform(input, options);

export let buildSync: typeof types.buildSync = (options: types.BuildOptions): any => {
// Try using a long-lived worker thread to avoid repeated start-up overhead
if (worker_threads) {
if (!workerThreadService) workerThreadService = startWorkerThreadService(worker_threads);
return workerThreadService.buildSync(options);
}

let result: types.BuildResult;
runServiceSync(service => service.buildOrServe('buildSync', null, null, options, isTTY(), process.cwd(), (err, res) => {
runServiceSync(service => service.buildOrServe('buildSync', null, null, options, isTTY(), defaultWD, (err, res) => {
if (err) throw err;
result = res as types.BuildResult;
}));
return result!;
};

export let transformSync: typeof types.transformSync = (input, options) => {
// Try using a long-lived worker thread to avoid repeated start-up overhead
if (worker_threads) {
if (!workerThreadService) workerThreadService = startWorkerThreadService(worker_threads);
return workerThreadService.transformSync(input, options);
}

let result: types.TransformResult;
runServiceSync(service => service.transform('transformSync', null, input, options || {}, isTTY(), {
readFile(tempFile, callback) {
Expand Down Expand Up @@ -136,6 +163,7 @@ let ensureServiceIsRunning = (): Service => {
let child = child_process.spawn(command, args.concat(`--service=${ESBUILD_VERSION}`, '--ping'), {
windowsHide: true,
stdio: ['pipe', 'pipe', 'inherit'],
cwd: defaultWD,
});

let { readFromStdout, afterClose, service } = common.createChannel({
Expand Down Expand Up @@ -235,7 +263,7 @@ let runServiceSync = (callback: (service: common.StreamService) => void): void =
});
callback(service);
let stdout = child_process.execFileSync(command, args.concat(`--service=${ESBUILD_VERSION}`), {
cwd: process.cwd(),
cwd: defaultWD,
windowsHide: true,
input: stdin,

Expand All @@ -252,3 +280,165 @@ let runServiceSync = (callback: (service: common.StreamService) => void): void =
let randomFileName = () => {
return path.join(os.tmpdir(), `esbuild-${crypto.randomBytes(32).toString('hex')}`);
};

interface MainToWorkerMessage {
sharedBuffer: SharedArrayBuffer;
id: number;
command: string;
args: any[];
}

interface WorkerThreadService {
buildSync(options: types.BuildOptions): types.BuildResult;
transformSync(input: string, options?: types.TransformOptions): types.TransformResult;
}

let workerThreadService: WorkerThreadService | null = null;

let startWorkerThreadService = (worker_threads: typeof import('worker_threads')): WorkerThreadService => {
let { port1: mainPort, port2: workerPort } = new worker_threads.MessageChannel();
let worker = new worker_threads.Worker(__filename, {
workerData: { workerPort, defaultWD },
transferList: [workerPort],

// From node's documentation: https://nodejs.org/api/worker_threads.html
//
// Take care when launching worker threads from preload scripts (scripts loaded
// and run using the `-r` command line flag). Unless the `execArgv` option is
// explicitly set, new Worker threads automatically inherit the command line flags
// from the running process and will preload the same preload scripts as the main
// thread. If the preload script unconditionally launches a worker thread, every
// thread spawned will spawn another until the application crashes.
//
execArgv: [],
});
let nextID = 0;
let wasStopped = false;

// This forbids options which would cause structured clone errors
let validateBuildSyncOptions = (options: types.BuildOptions | undefined): void => {
if (!options) return
let plugins = options.plugins
let incremental = options.incremental
if (plugins && plugins.length > 0) throw new Error(`Cannot use plugins in synchronous API calls`);
if (incremental) throw new Error(`Cannot use "incremental" with a synchronous build`);
};

// MessagePort doesn't copy the properties of Error objects. We still want
// error objects to have extra properties such as "warnings" so implement the
// property copying manually.
let applyProperties = (object: any, properties: Record<string, any>): void => {
for (let key in properties) {
object[key] = properties[key];
}
};

let runCallSync = (command: string, args: any[]): any => {
if (wasStopped) throw new Error('The service was stopped');
let id = nextID++;

// Make a fresh shared buffer for every request. That way we can't have a
// race where a notification from the previous call overlaps with this call.
let sharedBuffer = new SharedArrayBuffer(8);
let sharedBufferView = new Int32Array(sharedBuffer);

// Send the message to the worker. Note that the worker could potentially
// complete the request before this thread returns from this call.
let msg: MainToWorkerMessage = { sharedBuffer, id, command, args };
worker.postMessage(msg);

// If the value hasn't changed (i.e. the request hasn't been completed,
// wait until the worker thread notifies us that the request is complete).
//
// Otherwise, if the value has changed, the request has already been
// completed. Don't wait in that case because the notification may never
// arrive if it has already been sent.
let status = Atomics.wait(sharedBufferView, 0, 0);
if (status !== 'ok' && status !== 'not-equal') throw new Error('Internal error: Atomics.wait() failed: ' + status);

let { message: { id: id2, resolve, reject, properties } } = worker_threads!.receiveMessageOnPort(mainPort)!;
if (id !== id2) throw new Error(`Internal error: Expected id ${id} but got id ${id2}`);
if (reject) {
applyProperties(reject, properties);
throw reject;
}
return resolve;
};

// Calling unref() on a worker will allow the thread to exit if it's the last
// only active handle in the event system. This means node will still exit
// when there are no more event handlers from the main thread. So there's no
// need to have a "stop()" function.
worker.unref();

return {
buildSync(options) {
validateBuildSyncOptions(options);
return runCallSync('build', [options]);
},
transformSync(input, options) {
return runCallSync('transform', [input, options]);
},
};
};

let startSyncServiceWorker = () => {
let workerPort: import('worker_threads').MessagePort = worker_threads!.workerData.workerPort;
let parentPort = worker_threads!.parentPort!;
let service = ensureServiceIsRunning();

// Take the default working directory from the main thread because we want it
// to be consistent. This will be the working directory that was current at
// the time the "esbuild" package was first imported.
defaultWD = worker_threads!.workerData.defaultWD;

// MessagePort doesn't copy the properties of Error objects. We still want
// error objects to have extra properties such as "warnings" so implement the
// property copying manually.
let extractProperties = (object: any): Record<string, any> => {
let properties: Record<string, any> = {};
if (object && typeof object === 'object') {
for (let key in object) {
properties[key] = object[key];
}
}
return properties;
};

parentPort.on('message', (msg: MainToWorkerMessage) => {
(async () => {
let { sharedBuffer, id, command, args } = msg;
let sharedBufferView = new Int32Array(sharedBuffer);

try {
if (command === 'build') {
workerPort.postMessage({ id, resolve: await service.build(args[0]) });
} else if (command === 'transform') {
workerPort.postMessage({ id, resolve: await service.transform(args[0], args[1]) });
} else {
throw new Error(`Invalid command: ${command}`);
}
} catch (reject) {
workerPort.postMessage({ id, reject, properties: extractProperties(reject) });
}

// The message has already been posted by this point, so it should be
// safe to wake the main thread. The main thread should always get the
// message we sent above.

// First, change the shared value. That way if the main thread attempts
// to wait for us after this point, the wait will fail because the shared
// value has changed.
Atomics.add(sharedBufferView, 0, 1);

// Then, wake the main thread. This handles the case where the main
// thread was already waiting for us before the shared value was changed.
Atomics.notify(sharedBufferView, 0, Infinity);
})();
});
};

// If we're in the worker thread, start the worker code
if (worker_threads && !worker_threads.isMainThread) {
startSyncServiceWorker();
}
10 changes: 8 additions & 2 deletions scripts/register-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@ fs.mkdirSync(rootTestDir)

const entry = path.join(rootTestDir, 'entry.ts')
fs.writeFileSync(entry, `
console.log('worked' as string)
console.log('in entry.ts' as string)
require('./other.ts')
`)

const other = path.join(rootTestDir, 'other.ts')
fs.writeFileSync(other, `
console.log('in other.ts' as string)
`)

const register = path.join(rootTestDir, 'register.js')
Expand Down Expand Up @@ -42,7 +48,7 @@ async function main() {
})
await Promise.race([promise, wait])
clearTimeout(timeout)
assert.strictEqual(result, `worked\n`)
assert.strictEqual(result, `in entry.ts\nin other.ts\n`)
}

main().then(
Expand Down