Implement timeout argument in wasm2js_atomic_wait_i32#4385
Conversation
Also, fix bug where pointer was being used direcltly to index into Int32Array. I suppose this code had basically zero users until I tried to land this change in emscripten: emscripten-core/emscripten#15742
sbc100
force-pushed
the
wasm2js_atomic_wait_i32
branch
from
1d8ce69 to
f870efe
Compare
| if (timeoutHigh >= 0) { | ||
| // Convert from nanoseconds to milliseconds | ||
| // Taken from convertI32PairToI53 in emscripten's library_int53.js | ||
| timeout = ((timeoutLow / 1e6) >>> 0) + timeoutHigh * (4294967296 / 1e6); |
There was a problem hiding this comment.
I think maybe (timeoutLow / 1e6) >>> 0 should be (timeoutLow >>> 0) / 1e6. That would allow timeouts of less than 1ms as the current code rounds the output.
There was a problem hiding this comment.
I thought of that too at first, but it looks like the timeout cannot be sub-millisecond according to the specification. timeout is rational (ℝ) value:
- Let q be ? ToNumber(timeout).
- If q is NaN or +∞𝔽, let t be +∞; else if q is -∞𝔽, let t be 0; else let t be max(ℝ(q), 0).
There was a problem hiding this comment.
Hmm, maybe this differs between countries, but isn't ℝ the set of real numbers, not rational? See wikipedia, The rational numbers (Q) are included in the real numbers (R) (link)
JS does a ToNumber, so it converts to a double, and sub-millisecond values are allowed IIUC
There was a problem hiding this comment.
Ah, right! I got confused with natural (ℕ).
4 participants
Also, fix bug where pointer was being used direcltly to
index into Int32Array. I suppose this code had basically
zero users until I tried to land this change in emscripten:
emscripten-core/emscripten#15742