FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[RISC-V] Fix performance regression by kxxt · Pull Request #53412 · nodejs/node · GitHub

/ node Public

[RISC-V] Fix performance regression - #53412

Closed
kxxt wants to merge 2 commits into
nodejs:mainfrom
kxxt:riscv/fix-perf-regression
ConversationCommits (2)ChecksFiles changed
Closed

[RISC-V] Fix performance regression#53412
kxxt wants to merge 2 commits into
nodejs:mainfrom
kxxt:riscv/fix-perf-regression

Conversation

kxxt commented Jun 11, 2024

Copy link
Copy Markdown
Member

Fix a performance regression in riscv64 v8, as bisected in riscv-forks/electron#1. It affects nodejs 21, 22 and the main branch. Other branches are not affected.

kxxt added 2 commits June 11, 2024 13:25
deps: V8: cherry-pick 6ea594ff7132
089587d 
Original commit message:

    [riscv] Skip check sv57 when enable pointer compress

    Change-Id: I4332d3849d113af105630c0e20cd2b5e3deb9392
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5430889
    Commit-Queue: Ji Qiu <qiuji@iscas.ac.cn>
    Reviewed-by: Ji Qiu <qiuji@iscas.ac.cn>
    Cr-Commit-Position: refs/heads/main@{#93244}

Refs: v8/v8@6ea594f
deps: V8: cherry-pick a3cc8522a4c8
10d1566 
Original commit message:

    [riscv] avoid cpu probing in li_ptr

    CPU probing is an expensive thing to do and we should avoid doing it upon every li_ptr call.

    Fixes performance regresion bisected in riscv-forks/electron#1

    Change-Id: Ib5ff89b2a730e08de6735123ae60adeffe811ed8
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5612950
    Commit-Queue: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94349}

Refs: v8/v8@a3cc852

nodejs-github-bot commented Jun 11, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

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

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 Jun 11, 2024
kxxt mentioned this pull request Jun 11, 2024
Merged
gengjiawen added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 11, 2024
targos added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Jun 11, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 11, 2024

nodejs-github-bot commented Jun 11, 2024

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jun 11, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6029/

nodejs-github-bot commented Jun 11, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6029/

nodejs-github-bot commented Jun 11, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6029/

nodejs-github-bot commented Jun 11, 2024

Copy link
Copy Markdown
Collaborator

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

RafaelGSS commented Jun 11, 2024

Copy link
Copy Markdown
Member

FWIW v21 is now EOL, so it's unlikely to have this patch. It should land on main and v22 instead.

richardlau added the riscv64 Issues and PRs related to the riscv64 architecture. label Jun 11, 2024

nodejs-github-bot commented Jun 12, 2024

Copy link
Copy Markdown
Collaborator

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

richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 12, 2024
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 13, 2024
nodejs-github-bot pushed a commit that referenced this pull request Jun 13, 2024
deps: V8: cherry-pick 6ea594ff7132
7d90f06 
Original commit message:

    [riscv] Skip check sv57 when enable pointer compress

    Change-Id: I4332d3849d113af105630c0e20cd2b5e3deb9392
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5430889
    Commit-Queue: Ji Qiu <qiuji@iscas.ac.cn>
    Reviewed-by: Ji Qiu <qiuji@iscas.ac.cn>
    Cr-Commit-Position: refs/heads/main@{#93244}

Refs: v8/v8@6ea594f
PR-URL: #53412
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

nodejs-github-bot commented Jun 13, 2024

Copy link
Copy Markdown
Collaborator

Landed in 5a19a9b...3597070

nodejs-github-bot pushed a commit that referenced this pull request Jun 13, 2024
deps: V8: cherry-pick a3cc8522a4c8
3597070 
Original commit message:

    [riscv] avoid cpu probing in li_ptr

    CPU probing is an expensive thing to do and we should avoid doing it upon every li_ptr call.

    Fixes performance regresion bisected in riscv-forks/electron#1

    Change-Id: Ib5ff89b2a730e08de6735123ae60adeffe811ed8
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5612950
    Commit-Queue: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94349}

Refs: v8/v8@a3cc852
PR-URL: #53412
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Jun 20, 2024
deps: V8: cherry-pick 6ea594ff7132
2defaaf 
Original commit message:

    [riscv] Skip check sv57 when enable pointer compress

    Change-Id: I4332d3849d113af105630c0e20cd2b5e3deb9392
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5430889
    Commit-Queue: Ji Qiu <qiuji@iscas.ac.cn>
    Reviewed-by: Ji Qiu <qiuji@iscas.ac.cn>
    Cr-Commit-Position: refs/heads/main@{#93244}

Refs: v8/v8@6ea594f
PR-URL: #53412
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Jun 20, 2024
deps: V8: cherry-pick a3cc8522a4c8
bd68888 
Original commit message:

    [riscv] avoid cpu probing in li_ptr

    CPU probing is an expensive thing to do and we should avoid doing it upon every li_ptr call.

    Fixes performance regresion bisected in riscv-forks/electron#1

    Change-Id: Ib5ff89b2a730e08de6735123ae60adeffe811ed8
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5612950
    Commit-Queue: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94349}

Refs: v8/v8@a3cc852
PR-URL: #53412
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
deps: V8: cherry-pick 6ea594ff7132
31ef295 
Original commit message:

    [riscv] Skip check sv57 when enable pointer compress

    Change-Id: I4332d3849d113af105630c0e20cd2b5e3deb9392
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5430889
    Commit-Queue: Ji Qiu <qiuji@iscas.ac.cn>
    Reviewed-by: Ji Qiu <qiuji@iscas.ac.cn>
    Cr-Commit-Position: refs/heads/main@{#93244}

Refs: v8/v8@6ea594f
PR-URL: nodejs#53412
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
deps: V8: cherry-pick a3cc8522a4c8
fc9793d 
Original commit message:

    [riscv] avoid cpu probing in li_ptr

    CPU probing is an expensive thing to do and we should avoid doing it upon every li_ptr call.

    Fixes performance regresion bisected in riscv-forks/electron#1

    Change-Id: Ib5ff89b2a730e08de6735123ae60adeffe811ed8
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5612950
    Commit-Queue: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94349}

Refs: v8/v8@a3cc852
PR-URL: nodejs#53412
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos mentioned this pull request Jun 25, 2024
Merged
Karliz24 mentioned this pull request Jul 9, 2024
Open
targos pushed a commit to targos/node that referenced this pull request Jul 17, 2024
deps: V8: cherry-pick a3cc8522a4c8
b23f333 
Original commit message:

    [riscv] avoid cpu probing in li_ptr

    CPU probing is an expensive thing to do and we should avoid doing it upon every li_ptr call.

    Fixes performance regresion bisected in riscv-forks/electron#1

    Change-Id: Ib5ff89b2a730e08de6735123ae60adeffe811ed8
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5612950
    Commit-Queue: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Yahan Lu <yahan@iscas.ac.cn>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94349}

Refs: v8/v8@a3cc852
PR-URL: nodejs#53412
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
kxxt mentioned this pull request Jul 1, 2025
Open
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lpinca lpinca approved these changes
gengjiawen gengjiawen approved these changes
richardlau richardlau approved these changes
RafaelGSS RafaelGSS approved these changes
marco-ippolito marco-ippolito approved these changes
+1 more reviewer
targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. needs-ci PRs that need a full CI run. riscv64 Issues and PRs related to the riscv64 architecture. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL