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.

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

deps: V8: cherry-pick bc831f8ba33b #45788

Merged
merged 1 commit into from
Dec 10, 2022

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Dec 8, 2022

Referencing #45701, this commit from v8 can leverage different use cases (for example wasm), and I believe it shouldn't be blocked with my encodeInto experiment.

Original commit message:

[fastcall] Implement support for onebyte string arguments

This CL adds one byte string specialization support for fast API call arguments.
It introduces a kOneByteString variant to CTypeInfo.

We see a ~6x improvement in Deno's TextEncoder#encode microbenchmark.
Rendered results: https://divy-v8-patches.deno.dev/

Bug: chromium:1052746
Change-Id: I47c3a9e101cd18ddc6ad58f627db3a34231b60f7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4036884
Reviewed-by: Toon Verwaest <[email protected]>
Reviewed-by: Maya Lekova <[email protected]>
Commit-Queue: Maya Lekova <[email protected]>
Cr-Commit-Position: refs/heads/main@{#84552}

Refs: v8/v8@bc831f8

Original commit message:

    [fastcall] Implement support for onebyte string arguments

    This CL adds one byte string specialization support for fast API call arguments.
    It introduces a kOneByteString variant to CTypeInfo.

    We see a ~6x improvement in Deno's TextEncoder#encode microbenchmark.
    Rendered results: https://divy-v8-patches.deno.dev/

    Bug: chromium:1052746
    Change-Id: I47c3a9e101cd18ddc6ad58f627db3a34231b60f7
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4036884
    Reviewed-by: Toon Verwaest <[email protected]>
    Reviewed-by: Maya Lekova <[email protected]>
    Commit-Queue: Maya Lekova <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#84552}

Refs: v8/v8@bc831f8
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Dec 8, 2022
@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Dec 8, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 8, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 9, 2022
@gengjiawen gengjiawen added the fast-track PRs that do not need to wait for 48 hours to land. label Dec 9, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2022

Fast-track has been requested by @gengjiawen. Please 👍 to approve.

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 9, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/45788
✔  Done loading data for nodejs/node/pull/45788
----------------------------------- PR info ------------------------------------
Title      deps: V8: cherry-pick bc831f8ba33b (#45788)
Author     Yagiz Nizipli  (@anonrig)
Branch     anonrig:v8/fast-api-string -> nodejs:main
Labels     build, v8 engine, fast-track, author ready, needs-ci
Commits    1
 - deps: V8: cherry-pick bc831f8ba33b
Committers 1
 - Yagiz Nizipli 
PR-URL: https://github.com/nodejs/node/pull/45788
Refs: https://github.com/v8/v8/commit/bc831f8ba33b79e2eb670faf1f84c4e39aeb0f9f
Reviewed-By: Anna Henningsen 
Reviewed-By: Jiawen Geng 
Reviewed-By: Daeyeon Jeong 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/45788
Refs: https://github.com/v8/v8/commit/bc831f8ba33b79e2eb670faf1f84c4e39aeb0f9f
Reviewed-By: Anna Henningsen 
Reviewed-By: Jiawen Geng 
Reviewed-By: Daeyeon Jeong 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 08 Dec 2022 15:13:09 GMT
   ✔  Approvals: 3
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/45788#pullrequestreview-1210642814
   ✔  - Jiawen Geng (@gengjiawen): https://github.com/nodejs/node/pull/45788#pullrequestreview-1211011886
   ✔  - Daeyeon Jeong (@daeyeon): https://github.com/nodejs/node/pull/45788#pullrequestreview-1211148529
   ℹ  This PR is being fast-tracked
   ✖  Last GitHub CI failed
   ℹ  Last Full PR CI on 2022-12-08T23:08:17Z: https://ci.nodejs.org/job/node-test-pull-request/48360/
- Querying data for job/node-test-pull-request/48360/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3656535524

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Dec 9, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@gengjiawen gengjiawen added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 10, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 10, 2022
@nodejs-github-bot nodejs-github-bot merged commit 894aff7 into nodejs:main Dec 10, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 894aff7

ErickWendel pushed a commit to ErickWendel/node that referenced this pull request Dec 12, 2022
Original commit message:

    [fastcall] Implement support for onebyte string arguments

    This CL adds one byte string specialization support for fast API call arguments.
    It introduces a kOneByteString variant to CTypeInfo.

    We see a ~6x improvement in Deno's TextEncoder#encode microbenchmark.
    Rendered results: https://divy-v8-patches.deno.dev/

    Bug: chromium:1052746
    Change-Id: I47c3a9e101cd18ddc6ad58f627db3a34231b60f7
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4036884
    Reviewed-by: Toon Verwaest <[email protected]>
    Reviewed-by: Maya Lekova <[email protected]>
    Commit-Queue: Maya Lekova <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#84552}

Refs: v8/v8@bc831f8
PR-URL: nodejs#45788
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Daeyeon Jeong <[email protected]>
targos pushed a commit that referenced this pull request Jan 1, 2023
Original commit message:

    [fastcall] Implement support for onebyte string arguments

    This CL adds one byte string specialization support for fast API call arguments.
    It introduces a kOneByteString variant to CTypeInfo.

    We see a ~6x improvement in Deno's TextEncoder#encode microbenchmark.
    Rendered results: https://divy-v8-patches.deno.dev/

    Bug: chromium:1052746
    Change-Id: I47c3a9e101cd18ddc6ad58f627db3a34231b60f7
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4036884
    Reviewed-by: Toon Verwaest <[email protected]>
    Reviewed-by: Maya Lekova <[email protected]>
    Commit-Queue: Maya Lekova <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#84552}

Refs: v8/v8@bc831f8
PR-URL: #45788
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Daeyeon Jeong <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2023
@juanarbol
Copy link
Member

This needs manual backport to v18.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. build Issues and PRs related to build files or the CI. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants