-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
worker: support MessagePort to workers data #32278
Conversation
lib/internal/worker.js
Outdated
@@ -185,6 +185,8 @@ class Worker extends EventEmitter { | |||
this[kParentSideStdio] = { stdin, stdout, stderr }; | |||
|
|||
const { port1, port2 } = new MessageChannel(); | |||
const transferList = [port2]; | |||
if (options.transferList) transferList.push(...options.transferList); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (options.transferList) transferList.push(...options.transferList); | |
if (ArrayIsArray(options.transferList)) transferList.push(...options.transferList); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved! Thank you!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juanarbol @himself65 Hate to be late to this, but can we please undo this? I don’t see any reason why we wouldn’t accept any iterable here. We’re also quite flexible for the transferList
argument for .postMessage()
in accordance with the web spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem!
const transferList = [channel1.port1, channel2.port1]; | ||
new Worker(`${meowScript}`, { eval: true, workerData, transferList }); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{ | |
const uint8Array = new Uint8Array([ 1, 2, 3, 4 ]); | |
assert.deepStrictEqual(uint8Array.length, 4); | |
new Worker(` | |
const { parentPort, workerData } = require('worker_threads'); | |
parentPort.postMessage(workerData); | |
`, { | |
eval: true, | |
workerData: uint8Array, | |
transferList: [uint8Array.buffer] | |
}).on( | |
'message', | |
(message) => | |
assert.deepStrictEqual(message, Uint8Array.of(1, 2, 3, 4)) | |
); | |
assert.deepStrictEqual(uint8Array.length, 0); | |
} | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
semicolon, excuse me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, thanks for the the new test and semicolon is not required (make lint-js
passed on my machine)
ping @jasnell |
Looks good once @himself65 is also happy with it |
@jasnell Yes, LGTM |
@addaleax I've removed the array validation, with that change, the test for "INVALID_ARG_TYPE" is gone now, as long as we don't care now about if it is specifically an array or not. By the way, thanks for the review! |
CI: https://ci.nodejs.org/job/node-test-pull-request/30328/ (:heavy_check_mark:) |
By the way, I could work on backporting if needed. |
@juanarbol Two things…
Thank you! 👍 |
b06950a
to
7153abf
Compare
7153abf
to
787ec67
Compare
@addaleax Done! Thanks for the review, I've more about introducing new features to Node. |
Landed in ff2f47d 🙂 |
Notable changes: New file system APIs: * Added a new function, `fs.readv` (with sync and promisified versions). This function takes an array of `ArrayBufferView` elements and will write the data it reads sequentially to the buffers (Sk Sajidul Kadir). #32356 * A new overload is available for `fs.readSync`, which allows to optionally pass any of the `offset`, `length` and `position` parameters. #32460 Other changes: * dns: * Added the `dns.ALL` flag, that can be passed to `dns.lookup()` with `dns.V4MAPPED` to return resolved IPv6 addresses as well as IPv4 mapped IPv6 addresses (murgatroid99). #32183 * http: * The default maximum HTTP header size was changed from 8KB to 16KB (rosaxny). #32520 * n-api: * Calls to `napi_call_threadsafe_function` from the main thread can now return the `napi_would_deadlock` status in certain circumstances (Gabriel Schulhof). #32689 * util: * Added a new `maxStrLength` option to `util.inspect`, to control the maximum length of printed strings. Its default value is `Infinity` (rosaxny). #32392 * worker: * Added support for passing a `transferList` along with `workerData` to the `Worker` constructor (Juan José Arboleda). #32278 New core collaborators: With this release, we welcome three new Node.js core collaborators: * himself65. #32734 * flarna (Gerhard Stoebich). #32620 * mildsunrise (Alba Mendez). #32525 PR-URL: #32813
Notable changes: New file system APIs: * Added a new function, `fs.readv` (with sync and promisified versions). This function takes an array of `ArrayBufferView` elements and will write the data it reads sequentially to the buffers (Sk Sajidul Kadir). #32356 * A new overload is available for `fs.readSync`, which allows to optionally pass any of the `offset`, `length` and `position` parameters. #32460 Other changes: * dns: * Added the `dns.ALL` flag, that can be passed to `dns.lookup()` with `dns.V4MAPPED` to return resolved IPv6 addresses as well as IPv4 mapped IPv6 addresses (murgatroid99). #32183 * http: * The default maximum HTTP header size was changed from 8KB to 16KB (rosaxny). #32520 * n-api: * Calls to `napi_call_threadsafe_function` from the main thread can now return the `napi_would_deadlock` status in certain circumstances (Gabriel Schulhof). #32689 * util: * Added a new `maxStrLength` option to `util.inspect`, to control the maximum length of printed strings. Its default value is `Infinity` (rosaxny). #32392 * worker: * Added support for passing a `transferList` along with `workerData` to the `Worker` constructor (Juan José Arboleda). #32278 New core collaborators: With this release, we welcome three new Node.js core collaborators: * himself65. #32734 * flarna (Gerhard Stoebich). #32620 * mildsunrise (Alba Mendez). #32525 PR-URL: #32813
Notable changes: New file system APIs: * Added a new function, `fs.readv` (with sync and promisified versions). This function takes an array of `ArrayBufferView` elements and will write the data it reads sequentially to the buffers (Sk Sajidul Kadir). #32356 * A new overload is available for `fs.readSync`, which allows to optionally pass any of the `offset`, `length` and `position` parameters. #32460 Other changes: * dns: * Added the `dns.ALL` flag, that can be passed to `dns.lookup()` with `dns.V4MAPPED` to return resolved IPv6 addresses as well as IPv4 mapped IPv6 addresses (murgatroid99). #32183 * http: * The default maximum HTTP header size was changed from 8KB to 16KB (rosaxny). #32520 * n-api: * Calls to `napi_call_threadsafe_function` from the main thread can now return the `napi_would_deadlock` status in certain circumstances (Gabriel Schulhof). #32689 * util: * Added a new `maxStrLength` option to `util.inspect`, to control the maximum length of printed strings. Its default value is `Infinity` (rosaxny). #32392 * worker: * Added support for passing a `transferList` along with `workerData` to the `Worker` constructor (Juan José Arboleda). #32278 New core collaborators: With this release, we welcome three new Node.js core collaborators: * himself65. #32734 * flarna (Gerhard Stoebich). #32620 * mildsunrise (Alba Mendez). #32525 PR-URL: #32813
Hey, it seems the docs weren't really updated? |
Right … sorry, that seems to have been missed. @juanarbol I assume you can open a PR that updates the docs? |
Sure I can! |
PR-URL: nodejs#32278 Fixes: nodejs#32250 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
This adds support to send
MessagePort
-like objects within the WorkersworkerData
, the worker itself will be initialized with the passedMessagePort
instead of sendingport.postMessage(..., messagePort)
after initializing the worker.Fixes: #32250
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes