Skip to content

Commit 9c0efe9

Browse files
authored
Minor refactoring of library_wasm_worker.js/wasm_worker.h. NFC (#20109)
- Format comment blocks to 80 chars - Use {{{ cDefs }} rather than hardcoded numbers and comments - Rename atomicLiveWaitAsyncs and liveAtomicWaitCounter for clarity. Split out from #20099.
1 parent 76652b9 commit 9c0efe9

File tree

5 files changed

+323
-221
lines changed

5 files changed

+323
-221
lines changed

src/generated_struct_info32.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
"AL_STOPPED": 4116,
3838
"AL_TRUE": 1,
3939
"AL_VELOCITY": 4102,
40+
"ATOMICS_WAIT_NOT_EQUAL": 1,
41+
"ATOMICS_WAIT_TIMED_OUT": 2,
4042
"AT_EMPTY_PATH": 4096,
4143
"AT_FDCWD": -100,
4244
"AT_NO_AUTOMOUNT": 2048,

src/generated_struct_info64.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
"AL_STOPPED": 4116,
3838
"AL_TRUE": 1,
3939
"AL_VELOCITY": 4102,
40+
"ATOMICS_WAIT_NOT_EQUAL": 1,
41+
"ATOMICS_WAIT_TIMED_OUT": 2,
4042
"AT_EMPTY_PATH": 4096,
4143
"AT_FDCWD": -100,
4244
"AT_NO_AUTOMOUNT": 2048,

src/library_wasm_worker.js

Lines changed: 87 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,10 @@ addToLibrary({
3333
$_wasmWorkers: {},
3434
$_wasmWorkersID: 1,
3535

36-
// Starting up a Wasm Worker is an asynchronous operation, hence if the parent thread performs any
37-
// postMessage()-based wasm function calls s to the Worker, they must be delayed until the async
38-
// startup has finished, after which these postponed function calls can be dispatched.
36+
// Starting up a Wasm Worker is an asynchronous operation, hence if the parent
37+
// thread performs any postMessage()-based wasm function calls s to the
38+
// Worker, they must be delayed until the async startup has finished, after
39+
// which these postponed function calls can be dispatched.
3940
$_wasmWorkerDelayedMessageQueue: [],
4041

4142
$_wasmWorkerAppendToQueue: (e) => {
@@ -44,12 +45,14 @@ addToLibrary({
4445

4546
// Executes a wasm function call received via a postMessage.
4647
$_wasmWorkerRunPostMessage: (e) => {
47-
let data = e.data, wasmCall = data['_wsc']; // '_wsc' is short for 'wasm call', trying to use an identifier name that will never conflict with user code
48+
// '_wsc' is short for 'wasm call', trying to use an identifier name that
49+
// will never conflict with user code
50+
let data = e.data, wasmCall = data['_wsc'];
4851
wasmCall && getWasmTableEntry(wasmCall)(...data['x']);
4952
},
5053

51-
// src/postamble_minimal.js brings this symbol in to the build, and calls this function synchronously
52-
// from main JS file at the startup of each Worker.
54+
// src/postamble_minimal.js brings this symbol in to the build, and calls this
55+
// function synchronously from main JS file at the startup of each Worker.
5356
$_wasmWorkerInitializeRuntime__deps: ['$_wasmWorkerDelayedMessageQueue', '$_wasmWorkerRunPostMessage', '$_wasmWorkerAppendToQueue', 'emscripten_wasm_worker_initialize'],
5457
$_wasmWorkerInitializeRuntime: () => {
5558
let m = Module;
@@ -59,13 +62,13 @@ addToLibrary({
5962
#endif
6063

6164
#if STACK_OVERFLOW_CHECK >= 2
62-
// _emscripten_wasm_worker_initialize() initializes the stack for this Worker,
63-
// but it cannot call to extern __set_stack_limits() function, or Binaryen breaks
64-
// with "Fatal: Module::addFunction: __set_stack_limits already exists".
65-
// So for now, invoke this function from JS side. TODO: remove this in the future.
66-
// Note that this call is not exactly correct, since this limit will include
67-
// the TLS slot, that will be part of the region between m['sb'] and m['sz'],
68-
// so we need to fix up the call below.
65+
// _emscripten_wasm_worker_initialize() initializes the stack for this
66+
// Worker, but it cannot call to extern __set_stack_limits() function, or
67+
// Binaryen breaks with "Fatal: Module::addFunction: __set_stack_limits
68+
// already exists". So for now, invoke this function from JS side. TODO:
69+
// remove this in the future. Note that this call is not exactly correct,
70+
// since this limit will include the TLS slot, that will be part of the
71+
// region between m['sb'] and m['sz'], so we need to fix up the call below.
6972
___set_stack_limits(m['sb'] + m['sz'], m['sb']);
7073
#endif
7174
// Run the C side Worker initialization for stack and TLS.
@@ -100,33 +103,47 @@ addToLibrary({
100103
},
101104

102105
#if WASM_WORKERS == 2
103-
// In WASM_WORKERS == 2 build mode, we create the Wasm Worker global scope script from a string bundled in the main application JS file. This simplifies the number of deployed JS files with the app,
104-
// but has a downside that the generated build output will no longer be csp-eval compliant. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#unsafe_eval_expressions
106+
// In WASM_WORKERS == 2 build mode, we create the Wasm Worker global scope
107+
// script from a string bundled in the main application JS file. This
108+
// simplifies the number of deployed JS files with the app, but has a downside
109+
// that the generated build output will no longer be csp-eval compliant.
110+
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#unsafe_eval_expressions
105111
$_wasmWorkerBlobUrl: "URL.createObjectURL(new Blob(['onmessage=function(d){onmessage=null;d=d.data;{{{ captureModuleArg() }}}{{{ instantiateWasm() }}}importScripts(d.js);{{{ instantiateModule() }}}d.wasm=d.mem=d.js=0;}'],{type:'application/javascript'}))",
106112
#endif
107113

108-
_emscripten_create_wasm_worker__deps: ['$_wasmWorkers', '$_wasmWorkersID', '$_wasmWorkerAppendToQueue', '$_wasmWorkerRunPostMessage'
114+
_emscripten_create_wasm_worker__deps: [
115+
'$_wasmWorkers', '$_wasmWorkersID',
116+
'$_wasmWorkerAppendToQueue', '$_wasmWorkerRunPostMessage',
109117
#if WASM_WORKERS == 2
110-
, '$_wasmWorkerBlobUrl'
118+
'$_wasmWorkerBlobUrl',
111119
#endif
112120
],
113-
_emscripten_create_wasm_worker__postset: 'if (ENVIRONMENT_IS_WASM_WORKER) {\n'
114-
+ '_wasmWorkers[0] = this;\n'
115-
+ 'addEventListener("message", _wasmWorkerAppendToQueue);\n'
116-
+ '}\n',
121+
_emscripten_create_wasm_worker__postset: `
122+
if (ENVIRONMENT_IS_WASM_WORKER) {
123+
_wasmWorkers[0] = this;
124+
addEventListener("message", _wasmWorkerAppendToQueue);
125+
}`,
117126
_emscripten_create_wasm_worker: (stackLowestAddress, stackSize) => {
118127
let worker = _wasmWorkers[_wasmWorkersID] = new Worker(
119-
#if WASM_WORKERS == 2 // WASM_WORKERS=2 mode embeds .ww.js file contents into the main .js file as a Blob URL. (convenient, but not CSP security safe, since this is eval-like)
128+
#if WASM_WORKERS == 2
129+
// WASM_WORKERS=2 mode embeds .ww.js file contents into the main .js file
130+
// as a Blob URL. (convenient, but not CSP security safe, since this is
131+
// eval-like)
120132
_wasmWorkerBlobUrl
121-
#elif MINIMAL_RUNTIME // MINIMAL_RUNTIME has a structure where the .ww.js file is loaded from the main HTML file in parallel to all other files for best performance
133+
#elif MINIMAL_RUNTIME
134+
// MINIMAL_RUNTIME has a structure where the .ww.js file is loaded from
135+
// the main HTML file in parallel to all other files for best performance
122136
Module['$wb'] // $wb="Wasm worker Blob", abbreviated since not DCEable
123-
#else // default runtime loads the .ww.js file on demand.
137+
#else
138+
// default runtime loads the .ww.js file on demand.
124139
locateFile('{{{ WASM_WORKER_FILE }}}')
125140
#endif
126141
);
127142
// Craft the Module object for the Wasm Worker scope:
128143
worker.postMessage({
129-
'$ww': _wasmWorkersID, // Signal with a non-zero value that this Worker will be a Wasm Worker, and not the main browser thread.
144+
// Signal with a non-zero value that this Worker will be a Wasm Worker,
145+
// and not the main browser thread.
146+
'$ww': _wasmWorkersID,
130147
#if MINIMAL_RUNTIME
131148
'wasm': Module['wasm'],
132149
'js': Module['js'],
@@ -221,10 +238,12 @@ addToLibrary({
221238
// And at the time of writing, no other browser has it either.
222239
#if MIN_EDGE_VERSION < 91 || MIN_CHROME_VERSION < 91 || MIN_SAFARI_VERSION != TARGET_NOT_SUPPORTED || MIN_FIREFOX_VERSION != TARGET_NOT_SUPPORTED || ENVIRONMENT_MAY_BE_NODE
223240
// Partially polyfill Atomics.waitAsync() if not available in the browser.
224-
// Also polyfill for old Chrome-based browsers, where Atomics.waitAsync is broken until Chrome 91,
225-
// see https://bugs.chromium.org/p/chromium/issues/detail?id=1167541
241+
// Also polyfill for old Chrome-based browsers, where Atomics.waitAsync is
242+
// broken until Chrome 91, see
243+
// https://bugs.chromium.org/p/chromium/issues/detail?id=1167541
226244
// https://github.com/tc39/proposal-atomics-wait-async/blob/master/PROPOSAL.md
227-
// This polyfill performs polling with setTimeout() to observe a change in the target memory location.
245+
// This polyfill performs polling with setTimeout() to observe a change in the
246+
// target memory location.
228247
emscripten_atomic_wait_async__postset: `if (!Atomics.waitAsync || (typeof navigator !== 'undefined' && jstoi_q((navigator.userAgent.match(/Chrom(e|ium)\\/([0-9]+)\\./)||[])[2]) < 91)) {
229248
let __Atomics_waitAsyncAddresses = [/*[i32a, index, value, maxWaitMilliseconds, promiseResolve]*/];
230249
function __Atomics_pollWaitAsyncAddresses() {
@@ -261,74 +280,82 @@ Atomics.waitAsync = (i32a, index, value, maxWaitMilliseconds) => {
261280
};
262281
}`,
263282

264-
// These dependencies are artificial, issued so that we still get the waitAsync polyfill emitted
265-
// if code only calls emscripten_lock/semaphore_async_acquire()
266-
// but not emscripten_atomic_wait_async() directly.
283+
// These dependencies are artificial, issued so that we still get the
284+
// waitAsync polyfill emitted if code only calls
285+
// emscripten_lock/semaphore_async_acquire() but not
286+
// emscripten_atomic_wait_async() directly.
267287
emscripten_lock_async_acquire__deps: ['emscripten_atomic_wait_async'],
268288
emscripten_semaphore_async_acquire__deps: ['emscripten_atomic_wait_async'],
269289

270290
#endif
271291

272-
$atomicLiveWaitAsyncs: '{}',
273-
$atomicLiveWaitAsyncsCounter: '0',
292+
$liveAtomicWaitAsyncs: '{}',
293+
$liveAtomicWaitAsyncCounter: '0',
274294

275-
emscripten_atomic_wait_async__deps: ['$atomicWaitStates', '$atomicLiveWaitAsyncs', '$atomicLiveWaitAsyncsCounter', '$jstoi_q'],
295+
emscripten_atomic_wait_async__deps: ['$atomicWaitStates', '$liveAtomicWaitAsyncs', '$liveAtomicWaitAsyncCounter', '$jstoi_q'],
276296
emscripten_atomic_wait_async: (addr, val, asyncWaitFinished, userData, maxWaitMilliseconds) => {
277297
let wait = Atomics.waitAsync(HEAP32, addr >> 2, val, maxWaitMilliseconds);
278298
if (!wait.async) return atomicWaitStates.indexOf(wait.value);
279-
// Increment waitAsync generation counter, account for wraparound in case application does huge amounts of waitAsyncs per second (not sure if possible?)
299+
// Increment waitAsync generation counter, account for wraparound in case
300+
// application does huge amounts of waitAsyncs per second (not sure if
301+
// possible?)
280302
// Valid counterrange: 0...2^31-1
281-
let counter = atomicLiveWaitAsyncsCounter;
282-
atomicLiveWaitAsyncsCounter = Math.max(0, (atomicLiveWaitAsyncsCounter+1)|0);
283-
atomicLiveWaitAsyncs[counter] = addr >> 2;
303+
let counter = liveAtomicWaitAsyncCounter;
304+
liveAtomicWaitAsyncCounter = Math.max(0, (liveAtomicWaitAsyncCounter+1)|0);
305+
liveAtomicWaitAsyncs[counter] = addr >> 2;
284306
wait.value.then((value) => {
285-
if (atomicLiveWaitAsyncs[counter]) {
286-
delete atomicLiveWaitAsyncs[counter];
307+
if (liveAtomicWaitAsyncs[counter]) {
308+
delete liveAtomicWaitAsyncs[counter];
287309
{{{ makeDynCall('vpiip', 'asyncWaitFinished') }}}(addr, val, atomicWaitStates.indexOf(value), userData);
288310
}
289311
});
290312
return -counter;
291313
},
292314

293-
emscripten_atomic_cancel_wait_async__deps: ['$atomicLiveWaitAsyncs'],
315+
emscripten_atomic_cancel_wait_async__deps: ['$liveAtomicWaitAsyncs'],
294316
emscripten_atomic_cancel_wait_async: (waitToken) => {
295317
#if ASSERTIONS
296-
if (waitToken == 1 /* ATOMICS_WAIT_NOT_EQUAL */) warnOnce('Attempted to call emscripten_atomic_cancel_wait_async() with a value ATOMICS_WAIT_NOT_EQUAL (1) that is not a valid wait token! Check success in return value from call to emscripten_atomic_wait_async()');
297-
else if (waitToken == 2 /* ATOMICS_WAIT_TIMED_OUT */) warnOnce('Attempted to call emscripten_atomic_cancel_wait_async() with a value ATOMICS_WAIT_TIMED_OUT (2) that is not a valid wait token! Check success in return value from call to emscripten_atomic_wait_async()');
298-
else if (waitToken > 0) warnOnce('Attempted to call emscripten_atomic_cancel_wait_async() with an invalid wait token value ' + waitToken);
318+
if (waitToken == {{{ cDefs.ATOMICS_WAIT_NOT_EQUAL }}}) {
319+
warnOnce('Attempted to call emscripten_atomic_cancel_wait_async() with a value ATOMICS_WAIT_NOT_EQUAL (1) that is not a valid wait token! Check success in return value from call to emscripten_atomic_wait_async()');
320+
} else if (waitToken == {{{ cDefs.ATOMICS_WAIT_TIMED_OUT }}}) {
321+
warnOnce('Attempted to call emscripten_atomic_cancel_wait_async() with a value ATOMICS_WAIT_TIMED_OUT (2) that is not a valid wait token! Check success in return value from call to emscripten_atomic_wait_async()');
322+
} else if (waitToken > 0) {
323+
warnOnce(`Attempted to call emscripten_atomic_cancel_wait_async() with an invalid wait token value ${waitToken}`);
324+
}
299325
#endif
300-
if (atomicLiveWaitAsyncs[waitToken]) {
301-
// Notify the waitAsync waiters on the memory location, so that JavaScript garbage collection can occur
326+
if (liveAtomicWaitAsyncs[waitToken]) {
327+
// Notify the waitAsync waiters on the memory location, so that JavaScript
328+
// garbage collection can occur.
302329
// See https://github.com/WebAssembly/threads/issues/176
303-
// This has the unfortunate effect of causing spurious wakeup of all other waiters at the address (which
304-
// causes a small performance loss)
305-
Atomics.notify(HEAP32, atomicLiveWaitAsyncs[waitToken]);
306-
delete atomicLiveWaitAsyncs[waitToken];
307-
return 0 /* EMSCRIPTEN_RESULT_SUCCESS */;
330+
// This has the unfortunate effect of causing spurious wakeup of all other
331+
// waiters at the address (which causes a small performance loss).
332+
Atomics.notify(HEAP32, liveAtomicWaitAsyncs[waitToken]);
333+
delete liveAtomicWaitAsyncs[waitToken];
334+
return {{{ cDefs.EMSCRIPTEN_RESULT_SUCCESS }}};
308335
}
309336
// This waitToken does not exist.
310-
return -5 /* EMSCRIPTEN_RESULT_INVALID_PARAM */;
337+
return {{{ cDefs.EMSCRIPTEN_RESULT_INVALID_PARAM }}};
311338
},
312339

313-
emscripten_atomic_cancel_all_wait_asyncs__deps: ['$atomicLiveWaitAsyncs'],
340+
emscripten_atomic_cancel_all_wait_asyncs__deps: ['$liveAtomicWaitAsyncs'],
314341
emscripten_atomic_cancel_all_wait_asyncs: () => {
315-
let waitAsyncs = Object.values(atomicLiveWaitAsyncs);
342+
let waitAsyncs = Object.values(liveAtomicWaitAsyncs);
316343
waitAsyncs.forEach((address) => {
317344
Atomics.notify(HEAP32, address);
318345
});
319-
atomicLiveWaitAsyncs = {};
346+
liveAtomicWaitAsyncs = {};
320347
return waitAsyncs.length;
321348
},
322349

323-
emscripten_atomic_cancel_all_wait_asyncs_at_address__deps: ['$atomicLiveWaitAsyncs'],
350+
emscripten_atomic_cancel_all_wait_asyncs_at_address__deps: ['$liveAtomicWaitAsyncs'],
324351
emscripten_atomic_cancel_all_wait_asyncs_at_address: (address) => {
325352
address >>= 2;
326353
let numCancelled = 0;
327-
Object.keys(atomicLiveWaitAsyncs).forEach((waitToken) => {
328-
if (atomicLiveWaitAsyncs[waitToken] == address) {
354+
Object.keys(liveAtomicWaitAsyncs).forEach((waitToken) => {
355+
if (liveAtomicWaitAsyncs[waitToken] == address) {
329356
Atomics.notify(HEAP32, address);
330-
delete atomicLiveWaitAsyncs[waitToken];
331-
++numCancelled;
357+
delete liveAtomicWaitAsyncs[waitToken];
358+
numCancelled++;
332359
}
333360
});
334361
return numCancelled;

src/struct_info.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -967,6 +967,13 @@
967967
"EM_FUNC_SIG_PARAM_F2I"
968968
]
969969
},
970+
{
971+
"file": "emscripten/wasm_worker.h",
972+
"defines": [
973+
"ATOMICS_WAIT_NOT_EQUAL",
974+
"ATOMICS_WAIT_TIMED_OUT"
975+
]
976+
},
970977
{
971978
"file": "emscripten/emscripten.h",
972979
"defines": [

0 commit comments

Comments
 (0)