Skip to content
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.

Sign up for GitHub

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

Jump to bottom

wasi: fast calls #43697

Merged
merged 1 commit into from
Dec 8, 2022
Merged

wasi: fast calls #43697

merged 1 commit into from
Dec 8, 2022

Conversation

devsnek
Copy link
Member

@devsnek devsnek commented Jul 5, 2022 •
edited
Loading

implement fast calls for the wasi module.

the individual calls are about 50-100% faster, and testing with ripgrep compiled to wasm, it looks like real world programs are about 15-20% faster.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/v8-update
  • @nodejs/wasi

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 5, 2022
@devsnek devsnek force-pushed the wasi-fast-calls branch 5 times, most recently from 922d211 to b3f59cf Compare July 5, 2022 23:36
@devsnek devsnek added v8 engine Issues and PRs related to the V8 dependency. wasm Issues and PRs related to WebAssembly. v8 module Issues and PRs related to the "v8" subsystem. wasi Issues and PRs related to the WebAssembly System Interface. labels Jul 5, 2022
@devsnek
Copy link
Member Author

devsnek commented Jul 6, 2022

some hacky microbenchmarks i have say that this makes wasi calls ~2x faster. next i need to find some "real world" programs and see how those perform...

@devsnek devsnek force-pushed the wasi-fast-calls branch 2 times, most recently from fb6e1e4 to 0bf8869 Compare July 7, 2022 04:53
bnoordhuis
bnoordhuis reviewed Jul 7, 2022
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by comment.

src/node_wasi.cc Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

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

@devsnek devsnek marked this pull request as ready for review July 8, 2022 06:22
@targos
Copy link
Member

targos commented Jul 8, 2022

Please increment the embedder string in common.gypi once in each V8 backport commit.

@github-actions github-actions bot mentioned this pull request Jul 9, 2022
Open
21 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Dec 8, 2022
Open
8 tasks
@devsnek devsnek added commit-queue Add this label to land a pull request using GitHub Actions. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Dec 8, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 8, 2022
@nodejs-github-bot nodejs-github-bot merged commit b3bf07e into main Dec 8, 2022
@nodejs-github-bot nodejs-github-bot deleted the wasi-fast-calls branch December 8, 2022 04:22
@nodejs-github-bot
Copy link
Collaborator

Landed in b3bf07e

@github-actions github-actions bot mentioned this pull request Dec 9, 2022
Open
5 tasks
ErickWendel pushed a commit to ErickWendel/node that referenced this pull request Dec 12, 2022
@devsnek @ErickWendel
wasi: fast calls
4b6f01c 
PR-URL: nodejs#43697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Dec 12, 2022
@devsnek @targos
wasi: fast calls
1aa4a6b 
PR-URL: #43697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@targos targos mentioned this pull request Dec 12, 2022
Merged
@targos
Copy link
Member

targos commented Dec 13, 2022

I tried to include this in v19.3.0 but two wasi tests failed on ARM macs.

targos pushed a commit that referenced this pull request Dec 14, 2022
@devsnek @targos
wasi: fast calls
476670e 
PR-URL: #43697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@devsnek
Copy link
Member Author

devsnek commented Dec 19, 2022

@targos if you want to try, you should be able to backport this to 19 if you include v8/v8@bf0bd48

targos pushed a commit to targos/node that referenced this pull request Dec 19, 2022
@devsnek @targos
wasi: fast calls
3e67704 
PR-URL: nodejs#43697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Dec 28, 2022
@devsnek @targos
wasi: fast calls
7bd6a2c 
PR-URL: #43697
Backport-PR-URL: #45908
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2023
Merged
@juanarbol juanarbol added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Jan 26, 2023
@juanarbol
Copy link
Member

This is not landing cleanly in v18.x release line

@TerrorJack TerrorJack mentioned this pull request Feb 22, 2023
Open
@devsnek devsnek added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. and removed backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. labels Apr 2, 2023
@kateinoigakukun kateinoigakukun mentioned this pull request Jan 4, 2024
Open
@TerrorJack TerrorJack mentioned this pull request May 21, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis left review comments

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax Awaiting requested review from addaleax

@guybedford guybedford Awaiting requested review from guybedford

@targos targos Awaiting requested review from targos

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. wasi Issues and PRs related to the WebAssembly System Interface. wasm Issues and PRs related to WebAssembly.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@devsnek @nodejs-github-bot @targos @cjihrig @MayaLekova @juanarbol @bnoordhuis