Skip to content

Commit 7e3388c

Browse files
authored
Simplify Worker setup, and support .json.gz inputs in symbolicator-cli (#5556)
The symbolicator-cli script only supported .json files as inputs, not .json.gz or art traces etc. In Node, window.Worker is not defined, so making the symbolicator-cli/index.ts change without the gz.ts changes causes an error to be shown if .json.gz inputs are given. I also went ahead and simplified our worker mocking that we used in tests.
2 parents 65fbff3 + 318ad7f commit 7e3388c

File tree

16 files changed

+2180
-87
lines changed

16 files changed

+2180
-87
lines changed

babel.config.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@
6565
"module-resolver",
6666
{
6767
"alias": {
68-
"firefox-profiler": "./src"
68+
"firefox-profiler": "./src",
69+
"firefox-profiler-res": "./res"
6970
}
7071
}
7172
]

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@
122122
"@types/common-tags": "^1.8.4",
123123
"@types/jest": "^30.0.0",
124124
"@types/minimist": "^1.2.5",
125+
"@types/node": "^22.17.2",
125126
"@types/query-string": "^6.3.0",
126127
"@types/react": "^18.3.24",
127128
"@types/react-dom": "^18.3.1",

src/profile-logic/process-profile.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2001,7 +2001,7 @@ export async function unserializeProfileOfArbitraryFormat(
20012001
arbitraryFormat = convertSimpleperfTraceProfile(arrayBuffer);
20022002
} else {
20032003
try {
2004-
const textDecoder = new TextDecoder();
2004+
const textDecoder = new TextDecoder(undefined, { fatal: true });
20052005
arbitraryFormat = await textDecoder.decode(arrayBuffer);
20062006
} catch (e) {
20072007
console.error('Source exception:', e);

src/symbolicator-cli/index.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,21 @@ export interface CliOptions {
7979

8080
export async function run(options: CliOptions) {
8181
console.log(`Loading profile from ${options.input}`);
82-
const serializedProfile = JSON.parse(fs.readFileSync(options.input, 'utf8'));
83-
const profile = await unserializeProfileOfArbitraryFormat(serializedProfile);
82+
83+
// Read the raw bytes from the file. It might be a JSON file, but it could also
84+
// be a binary file, e.g. a .json.gz file, or any of the binary formats supported
85+
// by our importers.
86+
const bytes = fs.readFileSync(options.input, null);
87+
88+
// bytes is a Uint8Array whose underlying ArrayBuffer can be longer than bytes.length.
89+
// Copy the contents into a new ArrayBuffer which is sized correctly, so that we
90+
// don't include uninitialized data from the extra parts of the underlying buffer.
91+
// Alternatively, we could make unserializeProfileOfArbitraryFormat support
92+
// Uint8Array or Buffer in addition to ArrayBuffer.
93+
const byteBufferCopy = Uint8Array.prototype.slice.call(bytes).buffer;
94+
95+
// Load the profile.
96+
const profile = await unserializeProfileOfArbitraryFormat(byteBufferCopy);
8497
if (profile === undefined) {
8598
throw new Error('Unable to parse the profile.');
8699
}

src/symbolicator-cli/webpack.config.js

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,7 @@ module.exports = {
1717
module: {
1818
rules: [
1919
{
20-
test: /\.js$/,
21-
use: ['babel-loader'],
22-
include: includes,
23-
},
24-
{
25-
test: /\.(ts|tsx)$/,
20+
test: /\.(js|ts|tsx)$/,
2621
use: ['babel-loader'],
2722
include: includes,
2823
},
Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,44 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
import { Worker } from 'worker_threads';
5+
import {
6+
Worker as NodeWorkerClass,
7+
isMarkedAsUntransferable,
8+
} from 'worker_threads';
69

7-
class NodeWorker {
8-
_instance: Worker | null;
10+
function getWorkerScript(file: string): string {
11+
return `
12+
const { parentPort } = require('worker_threads');
13+
const fs = require('fs');
14+
const vm = require('vm');
15+
16+
const scriptContent = fs.readFileSync("${file}", 'utf8');
17+
18+
const sandbox = {
19+
importScripts: function () {
20+
throw new Error('The function "importScripts" is not implemented.');
21+
},
22+
postMessage: parentPort.postMessage.bind(parentPort),
23+
onmessage: function () {},
24+
};
25+
26+
vm.runInNewContext(scriptContent, sandbox, { filename: "${file}" });
27+
28+
parentPort.onmessage = sandbox.onmessage.bind(null);
29+
`;
30+
}
31+
32+
export class NodeWorker {
33+
_instance: NodeWorkerClass | null;
934
onmessage: ((event: MessageEvent) => unknown) | null;
1035

1136
constructor(file: string) {
12-
const worker = new Worker(__dirname + '/node-worker-contents.mjs', {
13-
workerData: file,
14-
});
37+
const worker = new NodeWorkerClass(getWorkerScript(file), { eval: true });
1538
worker.on('message', this.onMessage);
1639
worker.on('error', this.onError);
1740
this._instance = worker;
1841
this.onmessage = null;
42+
workerInstances.push(worker);
1943
}
2044

2145
postMessage(message: unknown, transfer?: any[]) {
@@ -31,7 +55,11 @@ class NodeWorker {
3155
if (+major < 11 || (+major === 11 && +minor < 12)) {
3256
payload = { data: message };
3357
}
34-
this._instance?.postMessage(payload, transfer);
58+
// See https://github.com/nodejs/node/issues/55593
59+
const actualTransfer = (transfer ?? []).filter(
60+
(buf) => !isMarkedAsUntransferable(buf)
61+
);
62+
this._instance?.postMessage(payload, actualTransfer);
3563
}
3664

3765
onMessage = (message: unknown) => {
@@ -53,23 +81,13 @@ class NodeWorker {
5381
}
5482
}
5583

56-
const workerConfigs: { [key: string]: string } = {
57-
'zee-worker': './res/zee-worker.js',
58-
};
59-
60-
const workerInstances: NodeWorker[] = [];
61-
62-
export default function (file: string): NodeWorker {
63-
const path = workerConfigs[file];
64-
const worker = new NodeWorker(path);
65-
workerInstances.push(worker);
66-
return worker;
67-
}
84+
const workerInstances: NodeWorkerClass[] = [];
6885

69-
/**
70-
* This function allows for stopping the workers, and is only part of the mock.
71-
*/
86+
// Called after running each test (see setup.js), otherwise Jest won't shut down.
7287
export function __shutdownWorkers() {
73-
workerInstances.forEach((worker) => worker.terminate());
88+
workerInstances.forEach((worker) => {
89+
worker.terminate();
90+
worker.unref();
91+
});
7492
workerInstances.length = 0;
7593
}

0 commit comments

Comments
 (0)