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

deps: V8: cherry-pick 9ebca66a5740 #53522

Merged
merged 1 commit into from
Jun 23, 2024
Merged

deps: V8: cherry-pick 9ebca66a5740 #53522

merged 1 commit into from
Jun 23, 2024

Conversation

legendecas
Copy link
Member

Original commit message:

[rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

Bug: v8:11111
Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66

--harmony-rab-gsab has been enabled for a while and there were no recent bug fixes (ref). Backport to unblock #50130.

/cc @anonrig

@legendecas
deps: V8: cherry-pick 9ebca66a5740
d3bc362 
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

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

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @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 Jun 20, 2024
@legendecas
Copy link
Member Author

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/6041/

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 20, 2024 •
edited by legendecas
Loading

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

It seems the CI failed for compiler OOM.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 20, 2024 •
edited by legendecas
Loading

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

It seems the CI failed for compiler OOM.

@anonrig anonrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 20, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 21, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 21, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 21, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 22, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/53522
✔  Done loading data for nodejs/node/pull/53522
----------------------------------- PR info ------------------------------------
Title      deps: V8: cherry-pick 9ebca66a5740 (#53522)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     legendecas:v8/rab-gasb -> nodejs:main
Labels     build, v8 engine, author ready, needs-ci
Commits    1
 - deps: V8: cherry-pick 9ebca66a5740
Committers 1
 - Chengzhong Wu 
PR-URL: https://github.com/nodejs/node/pull/53522
Refs: https://github.com/v8/v8/commit/9ebca66a57409dd9441868a9ce429c0c3f61409d
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Michaël Zasso 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53522
Refs: https://github.com/v8/v8/commit/9ebca66a57409dd9441868a9ce429c0c3f61409d
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Michaël Zasso 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 20 Jun 2024 22:50:33 GMT
   ✔  Approvals: 3
   ✔  - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/53522#pullrequestreview-2131574917
   ✔  - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/53522#pullrequestreview-2131851564
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/53522#pullrequestreview-2132973025
   ✘  Last GitHub CI failed
   ℹ  Last V8 CI on 2024-06-20T22:52:43Z: https://ci.nodejs.org/job/node-test-commit-v8-linux/6041/
   ℹ  Last Full PR CI on 2024-06-21T09:10:49Z: https://ci.nodejs.org/job/node-test-pull-request/59908/
- Querying data for job/node-test-pull-request/59908/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/9628833940

@nodejs-github-bot
Copy link
Collaborator

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

@VoltrexKeyva VoltrexKeyva 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 Jun 23, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 23, 2024
@nodejs-github-bot nodejs-github-bot merged commit 4c730ae into nodejs:main Jun 23, 2024
72 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 4c730ae

@legendecas legendecas deleted the v8/rab-gasb branch June 23, 2024 22:35
@joyeecheung
Copy link
Member

It seems this broke test-snapshot-reproducible:

Commit before: https://ci.nodejs.org/job/node-test-commit-linux-containered/44134/
Commit after: https://ci.nodejs.org/job/node-test-commit-linux-containered/44135/

@targos targos added the dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. label Jun 25, 2024
@legendecas
Copy link
Member Author

@joyeecheung Is the failure only reproducible on linux? I tested locally on mac and it didn't reproduce. Will try to find a linux machine to verify it.

@legendecas
Copy link
Member Author

This was reverted at #53582

joyeecheung pushed a commit to joyeecheung/node that referenced this pull request Jul 7, 2024
@legendecas @joyeecheung
deps: V8: cherry-pick 9ebca66a5740
7b774d6 
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66
PR-URL: nodejs#53522
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@legendecas legendecas mentioned this pull request Jul 7, 2024
Closed
nodejs-github-bot pushed a commit that referenced this pull request Jul 9, 2024
@legendecas @nodejs-github-bot
deps: V8: cherry-pick 9ebca66a5740
1a1639d 
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66
PR-URL: #53522
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #53755
Fixes: #53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 12, 2024
@legendecas @aduh95
deps: V8: cherry-pick 9ebca66a5740
f1c30a3 
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66
PR-URL: #53522
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #53755
Fixes: #53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 16, 2024
@legendecas @aduh95
deps: V8: cherry-pick 9ebca66a5740
8542ace 
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66
PR-URL: #53522
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #53755
Fixes: #53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ehsankhfr pushed a commit to ehsankhfr/node that referenced this pull request Jul 18, 2024
@legendecas @ehsankhfr
deps: V8: cherry-pick 9ebca66a5740
c115d3d 
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66
PR-URL: nodejs#53522
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#53755
Fixes: nodejs#53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@anonrig anonrig anonrig approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. needs-ci PRs that need a full CI run. 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.
7 participants
@legendecas @nodejs-github-bot @joyeecheung @lpinca @anonrig @targos @VoltrexKeyva