Skip to content

Conversation

@lukealbao
Copy link
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 <[email protected]> Commit-Queue: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/main@{#90501} Refs: v8/v8@f7d000a
@nodejs-github-bot
Copy link
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 Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. 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
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
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 <[email protected]> Commit-Queue: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/main@{#90501} Refs: v8/v8@f7d000a PR-URL: #50344 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]>
@targos
Copy link
Member

Landed in 5f852cc

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

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 <[email protected]> Commit-Queue: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/main@{#90501} Refs: v8/v8@f7d000a PR-URL: nodejs/node#50344 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]>
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 <[email protected]> Commit-Queue: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/main@{#90501} Refs: v8/v8@f7d000a PR-URL: nodejs/node#50344 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]>
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.v18.xIssues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

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