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: cherry-pick commits to be forward-compatible with V8 10.8 #44958

Closed
wants to merge 4 commits into from
Closed

deps: cherry-pick commits to be forward-compatible with V8 10.8 #44958

wants to merge 4 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Oct 11, 2022

  • deps: V8: cherry-pick 8b8703953616
  • deps: V8: cherry-pick 3d59a3c2c164
  • deps: V8: cherry-pick e7f0f26f5ef3
  • deps: V8: cherry-pick c3dffe6e2bda

@targos
deps: V8: cherry-pick 8b8703953616
842bd96 
Original commit message:

    [API] Remove second OnCriticalMemoryPressure

    Remove the deprecated OnCriticalMemoryPressure method with receives an
    informative parameter.

    [email protected]

    Bug: chromium:634547
    Change-Id: I932c3b5030291294dd340362f0b20d374e3067c0
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3780533
    Reviewed-by: Michael Lippautz <[email protected]>
    Commit-Queue: Clemens Backes <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#83254}

Refs: v8/v8@8b87039
@targos
deps: V8: cherry-pick 3d59a3c2c164
82fb1af 
Original commit message:

    Add option to report discarded allocations in sampling heap profiler

    A couple of customers have asked about using devtools to get information
    about temporary allocations, with the goal of reducing GC time and/or
    peak memory usage. Currently, the sampling heap profiler reports only
    objects which are still alive at the end of the profiling session. In
    this change, I propose adding configuration options when starting the
    sampling heap profiler so that it can optionally include information
    about objects which were discarded by the GC before the end of the
    profiling session. A user could run the sampling heap profiler in
    several different modes depending on their goals:

    1. To find memory leaks or determine which functions contribute most to
       steady-state memory consumption, the current default mode is best.
    2. To find functions which cause large temporary memory spikes or large
       GC pauses, the user can request data about both live objects and
       those collected by major GC.
    3. To tune for minimal GC activity in latency-sensitive applications
       like real-time audio processing, the user can request data about
       every allocation, including objects collected by major or minor GC.
    4. I'm not sure why anybody would want data about objects collected by
       minor GC and not objects collected by major GC, but it's also a valid
       flags combination.

    Change-Id: If55d5965a1de04fed3ae640a02ca369723f64fdf
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3868522
    Reviewed-by: Michael Lippautz <[email protected]>
    Reviewed-by: Camillo Bruni <[email protected]>
    Reviewed-by: Simon Zünd <[email protected]>
    Commit-Queue: Seth Brenith <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#83202}

Refs: v8/v8@3d59a3c
@targos
deps: V8: cherry-pick e7f0f26f5ef3
775476c 
Original commit message:

    Don't run sampling-heap-profiler-flags with stress-incremental-marking

    This test observes GC behavior and needs the garbage collector to work
    in a somewhat predictable way.

    Bug: v8:13286
    Change-Id: I24e6a4f33a644b5f1845cd34558da03fc196f7e5
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3898721
    Commit-Queue: Seth Brenith <[email protected]>
    Reviewed-by: Simon Zünd <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#83218}

Refs: v8/v8@e7f0f26
@targos
deps: V8: cherry-pick c3dffe6e2bda
ae1d02c 
Original commit message:

    [api] Expose parsed module source map urls

    Source map urls can be parsed from the magic comments. Expose them with
    public apis on the UnboundModuleScript, similar to the UnboundScript.

    Change-Id: Ia5dfdc8ff25f825c9fa7d241d0d79ba20028586b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3917379
    Reviewed-by: Camillo Bruni <[email protected]>
    Commit-Queue: Chengzhong Wu (legendecas) <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#83527}

Refs: v8/v8@c3dffe6
@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 Oct 11, 2022
@targos targos added dont-land-on-v14.x dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. labels Oct 11, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 11, 2022 •
edited by targos
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/47175/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/4905/

@nodejs-github-bot
Copy link
Collaborator

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

@RafaelGSS RafaelGSS mentioned this pull request Oct 11, 2022
Merged
@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Oct 12, 2022
Open
19 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@targos
Copy link
Member Author

targos commented Oct 12, 2022

Can I have more reviews? This PR is useless if it doesn't land before v19.0.0

@nodejs-github-bot
Copy link
Collaborator

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

@targos targos added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 12, 2022
@gengjiawen gengjiawen added the fast-track PRs that do not need to wait for 48 hours to land. label Oct 13, 2022
@github-actions
Copy link
Contributor

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

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

Commit Queue failed
- Loading data for nodejs/node/pull/44958
✔  Done loading data for nodejs/node/pull/44958
----------------------------------- PR info ------------------------------------
Title      deps: cherry-pick commits to be forward-compatible with V8 10.8 (#44958)
Author     Michaël Zasso  (@targos)
Branch     targos:v8-108-forward -> nodejs:main
Labels     build, v8 engine, fast-track, author ready, needs-ci, dont-land-on-v14.x, dont-land-on-v16.x, dont-land-on-v18.x
Commits    4
 - deps: V8: cherry-pick 8b8703953616
 - deps: V8: cherry-pick 3d59a3c2c164
 - deps: V8: cherry-pick e7f0f26f5ef3
 - deps: V8: cherry-pick c3dffe6e2bda
Committers 1
 - Michaël Zasso 
PR-URL: https://github.com/nodejs/node/pull/44958
Reviewed-By: Jiawen Geng 
Reviewed-By: Beth Griggs 
Reviewed-By: Rafael Gonzaga 
Reviewed-By: Richard Lau 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/44958
Reviewed-By: Jiawen Geng 
Reviewed-By: Beth Griggs 
Reviewed-By: Rafael Gonzaga 
Reviewed-By: Richard Lau 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 11 Oct 2022 07:11:02 GMT
   ✔  Approvals: 4
   ✔  - Jiawen Geng (@gengjiawen): https://github.com/nodejs/node/pull/44958#pullrequestreview-1136991828
   ✔  - Beth Griggs (@BethGriggs) (TSC): https://github.com/nodejs/node/pull/44958#pullrequestreview-1138944338
   ✔  - Rafael Gonzaga (@RafaelGSS): https://github.com/nodejs/node/pull/44958#pullrequestreview-1138997260
   ✔  - Richard Lau (@richardlau) (TSC): https://github.com/nodejs/node/pull/44958#pullrequestreview-1139133325
   ℹ  This PR is being fast-tracked
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-10-12T11:53:50Z: https://ci.nodejs.org/job/node-test-pull-request/47217/
   ℹ  Last V8 CI on 2022-10-11T07:12:45Z: https://ci.nodejs.org/job/node-test-commit-v8-linux/4905/
- Querying data for job/node-test-pull-request/47217/
   ✔  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 44958
From https://github.com/nodejs/node
 * branch                  refs/pull/44958/merge -> FETCH_HEAD
✔  Fetched commits as 214354fc9f3e..ae1d02cca04f
--------------------------------------------------------------------------------
[main 8994a7b20f] deps: V8: cherry-pick 8b8703953616
 Author: Michaël Zasso 
 Date: Tue Oct 11 09:00:17 2022 +0200
 2 files changed, 1 insertion(+), 13 deletions(-)
[main d3450ebe17] deps: V8: cherry-pick 3d59a3c2c164
 Author: Michaël Zasso 
 Date: Tue Oct 11 09:04:31 2022 +0200
 10 files changed, 135 insertions(+), 9 deletions(-)
 create mode 100644 deps/v8/test/inspector/heap-profiler/sampling-heap-profiler-flags-expected.txt
 create mode 100644 deps/v8/test/inspector/heap-profiler/sampling-heap-profiler-flags.js
[main 09ca7557e3] deps: V8: cherry-pick e7f0f26f5ef3
 Author: Michaël Zasso 
 Date: Tue Oct 11 09:05:51 2022 +0200
 2 files changed, 2 insertions(+), 1 deletion(-)
[main c25ae8ba0f] deps: V8: cherry-pick c3dffe6e2bda
 Author: Michaël Zasso 
 Date: Tue Oct 11 09:07:46 2022 +0200
 5 files changed, 95 insertions(+), 32 deletions(-)
   ✔  Patches applied
There are 4 commits in the PR. Attempting autorebase.
Rebasing (2/8)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

deps: V8: cherry-pick 8b8703953616


Original commit message:


[API] Remove second OnCriticalMemoryPressure

Remove the deprecated OnCriticalMemoryPressure method with receives an
informative parameter.

[email protected]

Bug: chromium:634547
Change-Id: I932c3b5030291294dd340362f0b20d374e3067c0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3780533
Reviewed-by: Michael Lippautz <[email protected]>
Commit-Queue: Clemens Backes <[email protected]>
Cr-Commit-Position: refs/heads/main@{#83254}



Refs: v8/v8@8b87039

PR-URL: #44958

Reviewed-By: Jiawen Geng [email protected]

Reviewed-By: Beth Griggs [email protected]

Reviewed-By: Rafael Gonzaga [email protected]

Reviewed-By: Richard Lau [email protected]


[detached HEAD 50bb902aff] deps: V8: cherry-pick 8b8703953616

Author: Michaël Zasso [email protected]

Date: Tue Oct 11 09:00:17 2022 +0200

2 files changed, 1 insertion(+), 13 deletions(-)

Rebasing (3/8)

Rebasing (4/8)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

deps: V8: cherry-pick 3d59a3c2c164


Original commit message:


Add option to report discarded allocations in sampling heap profiler

A couple of customers have asked about using devtools to get information
about temporary allocations, with the goal of reducing GC time and/or
peak memory usage. Currently, the sampling heap profiler reports only
objects which are still alive at the end of the profiling session. In
this change, I propose adding configuration options when starting the
sampling heap profiler so that it can optionally include information
about objects which were discarded by the GC before the end of the
profiling session. A user could run the sampling heap profiler in
several different modes depending on their goals:

1. To find memory leaks or determine which functions contribute most to
   steady-state memory consumption, the current default mode is best.
2. To find functions which cause large temporary memory spikes or large
   GC pauses, the user can request data about both live objects and
   those collected by major GC.
3. To tune for minimal GC activity in latency-sensitive applications
   like real-time audio processing, the user can request data about
   every allocation, including objects collected by major or minor GC.
4. I'm not sure why anybody would want data about objects collected by
   minor GC and not objects collected by major GC, but it's also a valid
   flags combination.

Change-Id: If55d5965a1de04fed3ae640a02ca369723f64fdf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3868522
Reviewed-by: Michael Lippautz <[email protected]>
Reviewed-by: Camillo Bruni <[email protected]>
Reviewed-by: Simon Zünd <[email protected]>
Commit-Queue: Seth Brenith <[email protected]>
Cr-Commit-Position: refs/heads/main@{#83202}



Refs: v8/v8@3d59a3c

PR-URL: #44958

Reviewed-By: Jiawen Geng [email protected]

Reviewed-By: Beth Griggs [email protected]

Reviewed-By: Rafael Gonzaga [email protected]

Reviewed-By: Richard Lau [email protected]


[detached HEAD 381ebdf623] deps: V8: cherry-pick 3d59a3c2c164

Author: Michaël Zasso [email protected]

Date: Tue Oct 11 09:04:31 2022 +0200

10 files changed, 135 insertions(+), 9 deletions(-)

create mode 100644 deps/v8/test/inspector/heap-profiler/sampling-heap-profiler-flags-expected.txt

create mode 100644 deps/v8/test/inspector/heap-profiler/sampling-heap-profiler-flags.js

Rebasing (5/8)

Rebasing (6/8)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

deps: V8: cherry-pick e7f0f26f5ef3


Original commit message:


Don't run sampling-heap-profiler-flags with stress-incremental-marking

This test observes GC behavior and needs the garbage collector to work
in a somewhat predictable way.

Bug: v8:13286
Change-Id: I24e6a4f33a644b5f1845cd34558da03fc196f7e5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3898721
Commit-Queue: Seth Brenith <[email protected]>
Reviewed-by: Simon Zünd <[email protected]>
Cr-Commit-Position: refs/heads/main@{#83218}



Refs: v8/v8@e7f0f26

PR-URL: #44958

Reviewed-By: Jiawen Geng [email protected]

Reviewed-By: Beth Griggs [email protected]

Reviewed-By: Rafael Gonzaga [email protected]

Reviewed-By: Richard Lau [email protected]


[detached HEAD f0ea968797] deps: V8: cherry-pick e7f0f26f5ef3

Author: Michaël Zasso [email protected]

Date: Tue Oct 11 09:05:51 2022 +0200

2 files changed, 2 insertions(+), 1 deletion(-)

Rebasing (7/8)

Rebasing (8/8)


Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

deps: V8: cherry-pick c3dffe6e2bda


Original commit message:


[api] Expose parsed module source map urls

Source map urls can be parsed from the magic comments. Expose them with
public apis on the UnboundModuleScript, similar to the UnboundScript.

Change-Id: Ia5dfdc8ff25f825c9fa7d241d0d79ba20028586b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3917379
Reviewed-by: Camillo Bruni <[email protected]>
Commit-Queue: Chengzhong Wu (legendecas) <[email protected]>
Cr-Commit-Position: refs/heads/main@{#83527}



Refs: v8/v8@c3dffe6

PR-URL: #44958

Reviewed-By: Jiawen Geng [email protected]

Reviewed-By: Beth Griggs [email protected]

Reviewed-By: Rafael Gonzaga [email protected]

Reviewed-By: Richard Lau [email protected]


[detached HEAD 34a361d23d] deps: V8: cherry-pick c3dffe6e2bda

Author: Michaël Zasso [email protected]

Date: Tue Oct 11 09:07:46 2022 +0200

5 files changed, 95 insertions(+), 32 deletions(-)


Successfully rebased and updated refs/heads/main.


ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.
https://github.com/nodejs/node/actions/runs/3240131344

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 13, 2022
@targos targos added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 13, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 13, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 214354f...4884746

nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2022
@targos @nodejs-github-bot
deps: V8: cherry-pick 8b8703953616
bab8b3a 
Original commit message:

    [API] Remove second OnCriticalMemoryPressure

    Remove the deprecated OnCriticalMemoryPressure method with receives an
    informative parameter.

    [email protected]

    Bug: chromium:634547
    Change-Id: I932c3b5030291294dd340362f0b20d374e3067c0
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3780533
    Reviewed-by: Michael Lippautz <[email protected]>
    Commit-Queue: Clemens Backes <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#83254}

Refs: v8/v8@8b87039
PR-URL: #44958
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2022
@targos @nodejs-github-bot
deps: V8: cherry-pick 3d59a3c2c164
690a837 
Original commit message:

    Add option to report discarded allocations in sampling heap profiler

    A couple of customers have asked about using devtools to get information
    about temporary allocations, with the goal of reducing GC time and/or
    peak memory usage. Currently, the sampling heap profiler reports only
    objects which are still alive at the end of the profiling session. In
    this change, I propose adding configuration options when starting the
    sampling heap profiler so that it can optionally include information
    about objects which were discarded by the GC before the end of the
    profiling session. A user could run the sampling heap profiler in
    several different modes depending on their goals:

    1. To find memory leaks or determine which functions contribute most to
       steady-state memory consumption, the current default mode is best.
    2. To find functions which cause large temporary memory spikes or large
       GC pauses, the user can request data about both live objects and
       those collected by major GC.
    3. To tune for minimal GC activity in latency-sensitive applications
       like real-time audio processing, the user can request data about
       every allocation, including objects collected by major or minor GC.
    4. I'm not sure why anybody would want data about objects collected by
       minor GC and not objects collected by major GC, but it's also a valid
       flags combination.

    Change-Id: If55d5965a1de04fed3ae640a02ca369723f64fdf
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3868522
    Reviewed-by: Michael Lippautz <[email protected]>
    Reviewed-by: Camillo Bruni <[email protected]>
    Reviewed-by: Simon Zünd <[email protected]>
    Commit-Queue: Seth Brenith <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#83202}

Refs: v8/v8@3d59a3c
PR-URL: #44958
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2022
@targos @nodejs-github-bot
deps: V8: cherry-pick e7f0f26f5ef3
34ba631 
Original commit message:

    Don't run sampling-heap-profiler-flags with stress-incremental-marking

    This test observes GC behavior and needs the garbage collector to work
    in a somewhat predictable way.

    Bug: v8:13286
    Change-Id: I24e6a4f33a644b5f1845cd34558da03fc196f7e5
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3898721
    Commit-Queue: Seth Brenith <[email protected]>
    Reviewed-by: Simon Zünd <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#83218}

Refs: v8/v8@e7f0f26
PR-URL: #44958
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2022
@targos @nodejs-github-bot
deps: V8: cherry-pick c3dffe6e2bda
4884746 
Original commit message:

    [api] Expose parsed module source map urls

    Source map urls can be parsed from the magic comments. Expose them with
    public apis on the UnboundModuleScript, similar to the UnboundScript.

    Change-Id: Ia5dfdc8ff25f825c9fa7d241d0d79ba20028586b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3917379
    Reviewed-by: Camillo Bruni <[email protected]>
    Commit-Queue: Chengzhong Wu (legendecas) <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#83527}

Refs: v8/v8@c3dffe6
PR-URL: #44958
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafaelGSS RafaelGSS RafaelGSS approved these changes

@BethGriggs BethGriggs BethGriggs approved these changes

@richardlau richardlau richardlau approved these changes

@gengjiawen gengjiawen gengjiawen 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. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@targos @nodejs-github-bot @gengjiawen @richardlau @BethGriggs @RafaelGSS