worker: support MessagePort to workers data#32278

juanarbol · 2020-03-15T00:36:46Z

This adds support to send MessagePort-like objects within the Workers workerData, the worker itself will be initialized with the passed MessagePort instead of sending port.postMessage(..., messagePort) after initializing the worker.

Fixes: #32250

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

himself65 · 2020-03-15T04:46:45Z

lib/internal/worker.js

Suggested change
if(options.transferList)transferList.push(...options.transferList);
if(ArrayIsArray(options.transferList))transferList.push(...options.transferList);

Improved! Thank you!!!

@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.

No problem!

himself65 · 2020-03-15T05:46:24Z

test/parallel/test-worker-workerdata-messageport.js

Suggested change
{
constuint8Array=newUint8Array([1,2,3,4]);
assert.deepStrictEqual(uint8Array.length,4);
newWorker(`
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);
}

semicolon, excuse me.

Nope, thanks for the the new test and semicolon is not required (make lint-js passed on my machine)

doc/api/worker_threads.md

lib/internal/worker.js

test/parallel/test-worker-workerdata-messageport.js

juanarbol · 2020-03-24T17:02:36Z

ping @jasnell

jasnell · 2020-03-24T17:09:19Z

Looks good once @himself65 is also happy with it

himself65 · 2020-03-24T17:12:08Z

@jasnell Yes, LGTM

juanarbol · 2020-04-01T16:37:44Z

@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!

nodejs-github-bot · 2020-04-01T16:47:32Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30328/ (:heavy_check_mark:)

juanarbol · 2020-04-01T19:02:38Z

By the way, I could work on backporting if needed.

addaleax · 2020-04-02T16:45:25Z

@juanarbol Two things…

Could you squash the commits and change the commit message here so that it starts with worker:, e.g. worker: support MessagePort in workerData? I’d have done that while landing, but I noticed that another thing needs to be changed too:
Can you add a changes: entry to the documentation for new Worker()? That should usually be done when adding new options.

Thank you! 👍

juanarbol · 2020-04-02T17:09:02Z

@addaleax Done! Thanks for the review, I've more about introducing new features to Node.

nodejs-github-bot · 2020-04-02T17:16:36Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30378/

PR-URL: #32278Fixes: #32250 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

addaleax · 2020-04-02T23:14:59Z

Landed in ff2f47d 🙂

PR-URL: #32278Fixes: #32250 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

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

SimonSchick · 2020-04-15T18:04:01Z

Hey, it seems the docs weren't really updated?

addaleax · 2020-04-15T19:10:35Z

Right … sorry, that seems to have been missed. @juanarbol I assume you can open a PR that updates the docs?

juanarbol · 2020-04-15T20:45:17Z

Sure I can!

Ref: nodejs#32278

PR-URL: nodejs#32278Fixes: nodejs#32250 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Ref: #32278 PR-URL: #32881 Refs: #32278 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

PR-URL: #32278Fixes: #32250 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Ref: #32278 PR-URL: #32881 Refs: #32278 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

nodejs-github-bot added the worker Issues and PRs related to Worker support. label Mar 15, 2020

himself65 requested changes Mar 15, 2020
View reviewed changes

himself65 reviewed Mar 15, 2020
View reviewed changes

jasnell reviewed Mar 16, 2020
View reviewed changes

doc/api/worker_threads.md Outdated Show resolvedHide resolved

jasnell reviewed Mar 16, 2020
View reviewed changes

lib/internal/worker.js Outdated Show resolvedHide resolved

jasnell reviewed Mar 16, 2020
View reviewed changes

test/parallel/test-worker-workerdata-messageport.js Outdated Show resolvedHide resolved

himself65 approved these changes Mar 30, 2020
View reviewed changes

addaleax approved these changes Apr 1, 2020
View reviewed changes

addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. semver-minor PRs that contain new features and should be released in the next minor version. labels Apr 1, 2020

jasnell approved these changes Apr 1, 2020
View reviewed changes

addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 2, 2020

juanarbol force-pushed the juanarbol/transfer-list-workers branch from b06950a to 7153abfCompare April 2, 2020 17:02

worker: support MessagePort to workers data
787ec67

juanarbol force-pushed the juanarbol/transfer-list-workers branch from 7153abf to 787ec67Compare April 2, 2020 17:06

addaleax approved these changes Apr 2, 2020
View reviewed changes

addaleax pushed a commit that referenced this pull request Apr 2, 2020
worker: support MessagePort to workers data
ff2f47d
PR-URL: #32278Fixes: #32250 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

addaleax closed this Apr 2, 2020

BethGriggs pushed a commit that referenced this pull request Apr 7, 2020
worker: support MessagePort to workers data
855e9bb
PR-URL: #32278Fixes: #32250 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

targos pushed a commit that referenced this pull request Apr 12, 2020
worker: support MessagePort to workers data
8cb5e41
PR-URL: #32278Fixes: #32250 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

targos mentioned this pull request Apr 13, 2020
v13.13.0 release proposal #32813
Merged

juanarbol added a commit to juanarbol/node that referenced this pull request Apr 16, 2020
doc: add documentation for transferList arg at worker threads
37e928c
Ref: nodejs#32278

juanarbol mentioned this pull request Apr 16, 2020
doc: add documentation for transferList arg at worker threads #32881
Closed
3 tasks

targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
worker: support MessagePort to workers data
0c30038
PR-URL: nodejs#32278Fixes: nodejs#32250 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

targos pushed a commit that referenced this pull request Apr 28, 2020
worker: support MessagePort to workers data
ca4e652
PR-URL: #32278Fixes: #32250 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

targos mentioned this pull request May 2, 2020
v12.17.0 release proposal #33197
Merged

juanarbol changed the title ~~lib: support MessagePort to workers data~~worker: support MessagePort to workers dataMay 7, 2020

juanarbol deleted the juanarbol/transfer-list-workers branch January 19, 2021 16:27

	if(options.transferList)transferList.push(...options.transferList);
	if(ArrayIsArray(options.transferList))transferList.push(...options.transferList);

+{
+constuint8Array=newUint8Array([1,2,3,4]);
+assert.deepStrictEqual(uint8Array.length,4);
+newWorker(`
+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);
+}

Uh oh!

worker: support MessagePort to workers data#32278

worker: support MessagePort to workers data #32278

Uh oh!

Conversation

juanarbol commented Mar 15, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

himself65Mar 15, 2020

Choose a reason for hiding this comment

Uh oh!

juanarbolMar 15, 2020

Choose a reason for hiding this comment

Uh oh!

addaleaxMar 29, 2020

Choose a reason for hiding this comment

Uh oh!

juanarbolMar 30, 2020

Choose a reason for hiding this comment

Uh oh!

himself65Mar 15, 2020

Choose a reason for hiding this comment

Uh oh!

himself65Mar 15, 2020

Choose a reason for hiding this comment

Uh oh!

juanarbolMar 15, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

juanarbol commented Mar 24, 2020

Uh oh!

jasnell commented Mar 24, 2020

Uh oh!

himself65 commented Mar 24, 2020

Uh oh!

juanarbol commented Apr 1, 2020

Uh oh!

nodejs-github-bot commented Apr 1, 2020• edited by addaleax Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juanarbol commented Apr 1, 2020

Uh oh!

addaleax commented Apr 2, 2020

Uh oh!

juanarbol commented Apr 2, 2020

Uh oh!

nodejs-github-bot commented Apr 2, 2020

Uh oh!

addaleax commented Apr 2, 2020

Uh oh!

SimonSchick commented Apr 15, 2020

Uh oh!

addaleax commented Apr 15, 2020

Uh oh!

juanarbol commented Apr 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

juanarbol commented Mar 15, 2020•
edited
Loading

nodejs-github-bot commented Apr 1, 2020•
edited by addaleax
Loading