Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
src: support V8 experimental shared values in messaging#47706
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
syg commented Apr 24, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
syg commented Apr 24, 2023
cc @rbuckton, I had meant to do this a while back but totally forgot. Once this merges then TSC can start playing around with the dev trial as well. |
debadree25 commented Apr 25, 2023
cc @nodejs/workers |
nodejs-github-bot commented Apr 25, 2023
syg commented Apr 26, 2023
@joyeecheung I don't know nodejs very well. Is the CI saying this PR is causing some test failures? I find that rather surprising. |
nodejs-github-bot commented Apr 26, 2023
nodejs-github-bot commented Apr 26, 2023
joyeecheung commented Apr 26, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
The test failures were probably just unmarked flakes. https://ci.nodejs.org/job/node-test-pull-request/51472/ passed (with marked flakes). |
addaleax left a comment
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.
I think adding a test would be valuable unless you are really certain that somebody will remember to add one once the feature's no longer experimental :)
syg commented Apr 27, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Edit: I added a test. |
nodejs-github-bot commented May 3, 2023
nodejs-github-bot commented May 4, 2023
syg commented May 5, 2023
@joyeecheung I still don't know how to read the CI reports. :) Does this look good to land? |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Joyee Cheung <[email protected]>
syg commented May 8, 2023
Good to merge? |
nodejs-github-bot commented May 8, 2023
joyeecheung commented May 8, 2023
Need to re-run the CI until it's green |
nodejs-github-bot commented May 9, 2023
Commit Queue failed- Loading data for nodejs/node/pull/47706 ✔ Done loading data for nodejs/node/pull/47706 ----------------------------------- PR info ------------------------------------ Title src: support V8 experimental shared values in messaging (#47706) Author Shu-yu Guo (@syg) Branch syg:shared-value-conveyor -> nodejs:main Labels c++, worker, needs-ci Commits 5 - src: support V8 experimental shared values in messaging - Add a test - Fix linter errors - Update test/parallel/test-experimental-shared-value-conveyor.js - Quell linter Committers 2 - Shu-yu Guo - GitHub PR-URL: https://github.com/nodejs/node/pull/47706 Reviewed-By: Joyee Cheung Reviewed-By: Anna Henningsen ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/47706 Reviewed-By: Joyee Cheung Reviewed-By: Anna Henningsen -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 24 Apr 2023 19:05:11 GMT ✔ Approvals: 2 ✔ - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/47706#pullrequestreview-1415757197 ✔ - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/47706#pullrequestreview-1404880614 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-05-08T16:44:58Z: https://ci.nodejs.org/job/node-test-pull-request/51702/ - Querying data for job/node-test-pull-request/51702/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 47706 From https://github.com/nodejs/node * branch refs/pull/47706/merge -> FETCH_HEAD ✔ Fetched commits as 0b3fcfcf351f..a0946e915825 -------------------------------------------------------------------------------- Auto-merging src/node_messaging.cc [main e400a95a4e] src: support V8 experimental shared values in messaging Author: Shu-yu Guo Date: Mon Apr 24 11:15:07 2023 -0700 2 files changed, 31 insertions(+), 4 deletions(-) [main 620a4f5057] Add a test Author: Shu-yu Guo Date: Tue May 2 13:49:57 2023 -0700 1 file changed, 23 insertions(+) create mode 100644 test/parallel/test-experimental-shared-value-conveyor.js [main 806c2bce27] Fix linter errors Author: Shu-yu Guo Date: Tue May 2 14:14:40 2023 -0700 1 file changed, 2 insertions(+), 2 deletions(-) [main d4b2962cae] Update test/parallel/test-experimental-shared-value-conveyor.js Author: Shu-yu Guo Date: Fri May 5 17:09:16 2023 -0400 1 file changed, 3 insertions(+), 1 deletion(-) [main 641c3af3b9] Quell linter Author: Shu-yu Guo Date: Fri May 5 15:05:18 2023 -0700 1 file changed, 1 insertion(+), 1 deletion(-) ✔ Patches applied There are 5 commits in the PR. Attempting autorebase. Rebasing (2/10)https://github.com/nodejs/node/actions/runs/4923112701 |
nodejs-github-bot commented May 9, 2023
Landed in b2f6eed |
PR-URL: #47706 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
This adds the missing integration support for the V8 experimental shared structs feature (
--harmony-struct). There is no observable difference to node if--harmony-structis not passed. The flag is false by default.This experimental feature is for proving out shared memory in JS. The standards side is tracked by https://github.com/tc39/proposal-structs, though the current experiment, for the sake of ease of experimentation, does not include syntax. The features in experiment is documented here.
I also intentionally did not add any tests because the features are experimental and volatile. Here is an illustrative example though: