[WIP] feat(sdk): use workers for inflight code #5547
Conversation
|
Thanks for opening this pull request! 🎉
|
|
Currently one blocker is this: Our API for cc @eladb |
|
Console preview environment is available at https://wing-console-pr-5547.fly.dev 🚀 Last Updated (UTC) 2024-01-26 00:05 |
|
My other concern with this approach is it seems to undo a bunch of our work when it comes to stack trace source mapping. For example, with this Wing code: bring cloud;
bring util;
let foo = inflight () => {
throw "uh oh!";
};
test "inflight stuff" {
foo();
}Running |
BenchmarksComparison to Baseline 🟥⬜⬜🟥⬜🟥⬜⬜⬜⬜⬜⬜⬜
⬜ Within 1.5 standard deviations Benchmarks may vary outside of normal expectations, especially when running in GitHub Actions CI. Results
Last Updated (UTC) 2024-01-26 00:11 |
| const worker = new Worker(shim, { | ||
| env: this.options.env, | ||
| eval: true, | ||
| }); |
There was a problem hiding this comment.
To fix sourcemaps, a more typical way to do this is to use a preload script instead of an eval'd shim:
const worker = new Worker(this.entrypoint, {
env: this.options.env,
// create a local file called sandbox-shim.ts with all the extra stuff needed
// This file will be executed first, then the entrypoint
execArgv: ["-r", require.resolve("./sandbox-shim")],
});| Object.defineProperty(globalThis, "__dirname", { | ||
| get: () => { throw new Error("__dirname cannot be used within bundled cloud functions"); }, | ||
| }); | ||
| Object.defineProperty(globalThis, "__filename", { | ||
| get: () => { throw new Error("__filename cannot be used within bundled cloud functions"); }, | ||
| }); |
There was a problem hiding this comment.
Doesn't esbuild already fail if we have any __dirname or __filename?
There was a problem hiding this comment.
This is actually a major limitation we have today. There's a good chance that existing user code will depend on these.
Any thoughts on how to address this?
There was a problem hiding this comment.
@MarkMcCulloh From what I can tell esbuild doesn't reject code with __dirname, it just silently leaves it (leading to possibly wrong runtime behavior).
It looks like it could be possible to support it through an esbuild plugin? evanw/esbuild#859
| // it could be better to keep the worker alive and reuse it | ||
| // but this requires additional work to make sure logs between invocations | ||
| // are not mixed up, and timeouts are handled correctly | ||
| const worker = new Worker(shim, { |
|
re: Workers vs separate node process -- I did a naive benchmark (scripts below) and it seemed at least superficially like their performance was roughly the same. So I can't think of any strong reasons against it off the top of my head // bench-workers.js
const { Worker } = require('worker_threads');
const numThreads = 10; // Number of worker threads to spawn
let threadsStarted = 0;
const start = process.hrtime.bigint();
for (let i = 0; i < numThreads; i++) {
const worker = new Worker(`
const { parentPort } = require('worker_threads');
parentPort.postMessage('Worker thread started');
`, { eval: true });
worker.on('message', () => {
threadsStarted++;
if (threadsStarted === numThreads) {
const end = process.hrtime.bigint();
console.log(`All ${numThreads} worker threads started in ${(end - start) / BigInt(1e6)} ms`);
}
});
}
// bench-process.js
const { fork } = require('child_process');
const numProcesses = 10; // Number of child processes to spawn
let processesStarted = 0;
const start = process.hrtime.bigint();
for (let i = 0; i < numProcesses; i++) {
const child = fork('./childProcess.js');
child.on('message', (msg) => {
if (msg === 'Child process started') {
processesStarted++;
if (processesStarted === numProcesses) {
const end = process.hrtime.bigint();
console.log(`All ${numProcesses} child processes started in ${(end - start) / BigInt(1e6)} ms`);
}
}
});
}
// childProcess.js
process.send('Child process started'); |
|
Closing in favor of #5554 |
Implementing one approach from #4725
Checklist
pr/e2e-fulllabel if this feature requires end-to-end testingBy submitting this pull request, I confirm that my contribution is made under the terms of the Wing Cloud Contribution License.