Skip to content

[v18 backport] deps: V8: cherry-pick f7d000a7ae7b - #50344

Closed
lukealbao wants to merge 1 commit into
nodejs:v18.x-stagingfrom
lukealbao:perf-basic-logging-fix-v18
Closed

[v18 backport] deps: V8: cherry-pick f7d000a7ae7b#50344
lukealbao wants to merge 1 commit into
nodejs:v18.x-stagingfrom
lukealbao:perf-basic-logging-fix-v18

Conversation

@lukealbao

Copy link
Copy Markdown
Contributor

This backports #50302, which can't be cleanly applied to the v18 branch. This is also priority for #50225, as we are on v18.

Original commit message:
[logging] Bugfix: LinuxPerfBasicLogger should log JS functions
This patch fixes a typo that was introduced in commit
c51041f45400928cd64fbc8f389c0dd0dd15f82f /
https://chromium-review.googlesource.com/c/v8/v8/+/2336793, which reversed the behavior of the perf_basic_prof_only_functions flag.
This also refactors the equivalent guard in LinuxPerfJitLogger to use the same inline CodeKind API for identifying JS Functions. This is unrelated to the bug, but it seems a fair rider to add on here.
Bug: v8:14387
Change-Id: I25766b0d45f4c65dfec5ae01e094a1ed94111054
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4954225
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#90501}
Refs: v8/v8@f7d000a
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

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

@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v18.x v8 engine Issues and PRs related to the V8 dependency. labels Oct 23, 2023
@targostargos added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 28, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 28, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

targos pushed a commit that referenced this pull request Oct 29, 2023
Original commit message:
[logging] Bugfix: LinuxPerfBasicLogger should log JS functions
This patch fixes a typo that was introduced in commit
c51041f45400928cd64fbc8f389c0dd0dd15f82f /
https://chromium-review.googlesource.com/c/v8/v8/+/2336793, which reversed the behavior of the perf_basic_prof_only_functions flag.
This also refactors the equivalent guard in LinuxPerfJitLogger to use the same inline CodeKind API for identifying JS Functions. This is unrelated to the bug, but it seems a fair rider to add on here.
Bug: v8:14387
Change-Id: I25766b0d45f4c65dfec5ae01e094a1ed94111054
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4954225
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#90501}
Refs: v8/v8@f7d000a
PR-URL: #50344
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@targos

Copy link
Copy Markdown
Member

Landed in 5f852cc

@targostargos closed this Oct 29, 2023
@lukealbao
lukealbao deleted the perf-basic-logging-fix-v18 branch November 15, 2023 19:16
@brancz

Copy link
Copy Markdown

Is there an indication of when there will be a v18 and v20 releases available with this? (don't know the nodejs release schedule very well, whether it's a cadence or just whenever there are bugs to be released)

@targostargos mentioned this pull request Nov 28, 2023
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Original commit message:
[logging] Bugfix: LinuxPerfBasicLogger should log JS functions
This patch fixes a typo that was introduced in commit
c51041f45400928cd64fbc8f389c0dd0dd15f82f /
https://chromium-review.googlesource.com/c/v8/v8/+/2336793, which reversed the behavior of the perf_basic_prof_only_functions flag.
This also refactors the equivalent guard in LinuxPerfJitLogger to use the same inline CodeKind API for identifying JS Functions. This is unrelated to the bug, but it seems a fair rider to add on here.
Bug: v8:14387
Change-Id: I25766b0d45f4c65dfec5ae01e094a1ed94111054
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4954225
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#90501}
Refs: v8/v8@f7d000a
PR-URL: nodejs/node#50344
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Original commit message:
[logging] Bugfix: LinuxPerfBasicLogger should log JS functions
This patch fixes a typo that was introduced in commit
c51041f45400928cd64fbc8f389c0dd0dd15f82f /
https://chromium-review.googlesource.com/c/v8/v8/+/2336793, which reversed the behavior of the perf_basic_prof_only_functions flag.
This also refactors the equivalent guard in LinuxPerfJitLogger to use the same inline CodeKind API for identifying JS Functions. This is unrelated to the bug, but it seems a fair rider to add on here.
Bug: v8:14387
Change-Id: I25766b0d45f4c65dfec5ae01e094a1ed94111054
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4954225
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#90501}
Refs: v8/v8@f7d000a
PR-URL: nodejs/node#50344
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.needs-ciPRs that need a full CI run.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@lukealbao@nodejs-github-bot@targos@brancz@richardlau