Skip to content

Commit

Permalink
Revert "fix evanw#595: revert the "worker_threads" change"
Browse files Browse the repository at this point in the history
This reverts commit e828322.
  • Loading branch information
cspotcode committed Dec 17, 2020
1 parent 5c22b6c commit 3ef2154
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 72 deletions.
3 changes: 0 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ jobs:
- name: npm ci
run: cd scripts && npm ci

- name: Register Test
run: node scripts/register-test.js

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

Expand Down
6 changes: 1 addition & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test:
make -j6 test-common

# These tests are for development
test-common: test-go vet-go verify-source-map end-to-end-tests js-api-tests plugin-tests register-test
test-common: test-go vet-go verify-source-map end-to-end-tests js-api-tests plugin-tests

# These tests are for release (the extra tests are not included in "test" because they are pretty slow)
test-all:
Expand Down Expand Up @@ -39,10 +39,6 @@ test-wasm-node: platform-wasm
test-wasm-browser: platform-wasm | scripts/browser/node_modules
cd scripts/browser && node browser-tests.js

register-test: cmd/esbuild/version.go | scripts/node_modules
cd npm/esbuild && npm version "$(ESBUILD_VERSION)" --allow-same-version
node scripts/register-test.js

verify-source-map: cmd/esbuild/version.go | scripts/node_modules
cd npm/esbuild && npm version "$(ESBUILD_VERSION)" --allow-same-version
node scripts/verify-source-map.js
Expand Down
176 changes: 176 additions & 0 deletions lib/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,21 @@ import fs = require('fs');
import os = require('os');
import tty = require('tty');

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

// There appears to be some some weird Windows-specific issue that non-
// deterministically causes tests that use the "worker_threads" library to fail
// to exit. This only happens in GitHub CI. I have not been able to reproduce
// this myself. Let's disable this optimization on Windows for now. This means
// synchronous API calls will potentially be ~10x slower on Windows.
if (os.platform() !== 'win32') {
// Don't crash if the "worker_threads" library isn't present
try {
worker_threads = require('worker_threads');
} catch {
}
}

declare const ESBUILD_VERSION: string;

// This file is used for both the "esbuild" package and the "esbuild-wasm"
Expand Down Expand Up @@ -94,6 +109,13 @@ export let transform: typeof types.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);
}

// Otherwise, fall back to running a dedicated child process
let result: types.BuildResult;
runServiceSync(service => service.buildOrServe('buildSync', null, options, isTTY(), (err, res) => {
if (err) throw err;
Expand All @@ -104,6 +126,14 @@ export let buildSync: typeof types.buildSync = (options: types.BuildOptions): an

export let transformSync: typeof types.transformSync = (input, options) => {
input += '';

// 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);
}

// Otherwise, fall back to running a dedicated child process
let result: types.TransformResult;
runServiceSync(service => service.transform('transformSync', input, options || {}, isTTY(), {
readFile(tempFile, callback) {
Expand Down Expand Up @@ -230,3 +260,149 @@ 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,
transferList: [workerPort],
});
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;
let parentPort = worker_threads!.parentPort!;
let servicePromise = startService();

// 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) => {
servicePromise.then(async (service) => {
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();
}
5 changes: 1 addition & 4 deletions scripts/esbuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,7 @@ exports.installForTests = dir => {
childProcess.execSync(`npm install --silent --no-audit --progress=false esbuild-${version}.tgz`, { cwd: installDir, env, stdio: 'inherit' })

// Evaluate the code
const ESBUILD_PACKAGE_PATH = path.join(installDir, 'node_modules', 'esbuild')
const mod = require(ESBUILD_PACKAGE_PATH)
mod.ESBUILD_PACKAGE_PATH = ESBUILD_PACKAGE_PATH
return mod
return require(path.join(installDir, 'node_modules', 'esbuild'))
}

// This is helpful for ES6 modules which don't have access to __dirname
Expand Down
60 changes: 0 additions & 60 deletions scripts/register-test.js

This file was deleted.

0 comments on commit 3ef2154

Please sign in to comment.