Skip to content

deps: V8: cherry-pick f7d000a7ae7b #50302

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 23, 2023
Merged

deps: V8: cherry-pick f7d000a7ae7b #50302

merged 1 commit into from
Oct 23, 2023

Conversation

lukealbao
Copy link
Contributor

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

@lukealbao
deps: V8: cherry-pick f7d000a7ae7b
8254883 
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
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 20, 2023
@lukealbao
Copy link
Contributor Author

This was not a clean apply as there was a conflict in AUTHORS that I had to resolve.

@lukealbao
Copy link
Contributor Author

Fixes #50225

@H4ad H4ad linked an issue Oct 20, 2023 that may be closed by this pull request
--perf-basic-prof-only-functions flag is broken since v8 8.6 #50225
Closed
@H4ad H4ad added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 20, 2023
@H4ad
Copy link
Member

H4ad commented Oct 20, 2023

And congratulations on your first PR Node and on V8 🎉

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 20, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 20, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@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/5624/

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

This was referenced Oct 21, 2023
Open
Open
Open
@nodejs-github-bot
Copy link
Collaborator

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

@debadree25 debadree25 added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 23, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 23, 2023
@nodejs-github-bot nodejs-github-bot merged commit f6f681b into nodejs:main Oct 23, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in f6f681b

@lukealbao lukealbao deleted the perf-basic-logging-fix branch October 23, 2023 15:51
This was referenced Oct 23, 2023
Closed
Closed
targos pushed a commit that referenced this pull request Oct 24, 2023
@lukealbao @targos
deps: V8: cherry-pick f7d000a7ae7b
f4da308 
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: #50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
@targos targos mentioned this pull request Oct 24, 2023
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@lukealbao @alexfernandez
deps: V8: cherry-pick f7d000a7ae7b
7906cad 
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#50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
@targos targos added dont-land-on-v18.x dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Nov 11, 2023
targos pushed a commit to targos/node that referenced this pull request Dec 18, 2023
@lukealbao @targos
deps: V8: cherry-pick f7d000a7ae7b
1c72faf 
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#50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Dec 18, 2023
@lukealbao @targos
deps: V8: cherry-pick f7d000a7ae7b
f51ba33 
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#50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Dec 21, 2023
@lukealbao @targos
deps: V8: cherry-pick f7d000a7ae7b
5de006e 
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#50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Dec 21, 2023
@lukealbao @targos
deps: V8: cherry-pick f7d000a7ae7b
559a108 
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#50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Dec 22, 2023
@lukealbao @targos
deps: V8: cherry-pick f7d000a7ae7b
0f1d543 
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#50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Dec 22, 2023
@lukealbao @targos
deps: V8: cherry-pick f7d000a7ae7b
fb9d643 
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#50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
targos pushed a commit that referenced this pull request Jan 4, 2024
@lukealbao @targos
deps: V8: cherry-pick f7d000a7ae7b
b682e7f 
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: #50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
PR-URL: #50115
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Medhansh404 pushed a commit to Medhansh404/node that referenced this pull request Jan 19, 2024
@lukealbao @Medhansh404
deps: V8: cherry-pick f7d000a7ae7b
b8f5df5 
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#50302
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
PR-URL: nodejs#50115
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@H4ad H4ad H4ad approved these changes

@debadree25 debadree25 debadree25 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-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.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.

--perf-basic-prof-only-functions flag is broken since v8 8.6
6 participants
@lukealbao @nodejs-github-bot @H4ad @debadree25 @targos @richardlau