Skip to content
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

[v8.x] deps: V8: cherry-pick 525b396 #25041

Closed
wants to merge 1 commit into from
Closed

[v8.x] deps: V8: cherry-pick 525b396 #25041

wants to merge 1 commit into from

Conversation

psmarshall
Copy link
Contributor

Original commit message:

[cpu-profiler] Fix a leak caused by re-logging existing functions.

Don't re-log all existing functions during StartProcessorIfNotStarted().
They will already be in the CodeMap attached to the ProfileGenerator and
re-logging them causes leaks. See the linked bug for more details.

Bug: v8:8253
Change-Id: Ibb1a1ab2431c588e8c3a3a9ff714767cdf61a88e
Reviewed-on: https://chromium-review.googlesource.com/1256763
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56336}

Refs: v8/v8@525b396

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@psmarshall
deps: V8: cherry-pick 525b396
bdd5f40 
Original commit message:

    [cpu-profiler] Fix a leak caused by re-logging existing functions.

    Don't re-log all existing functions during StartProcessorIfNotStarted().
    They will already be in the CodeMap attached to the ProfileGenerator and
    re-logging them causes leaks. See the linked bug for more details.

    Bug: v8:8253
    Change-Id: Ibb1a1ab2431c588e8c3a3a9ff714767cdf61a88e
    Reviewed-on: https://chromium-review.googlesource.com/1256763
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#56336}

Refs: v8/v8@525b396
@nodejs-github-bot
Copy link
Collaborator

@psmarshall build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1946/pipeline

@nodejs-github-bot nodejs-github-bot added v8 engine Issues and PRs related to the V8 dependency. v8.x labels Dec 14, 2018
@psmarshall psmarshall mentioned this pull request Dec 14, 2018
Closed
@psmarshall
Copy link
Contributor Author

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

@psmarshall psmarshall changed the title deps: V8: cherry-pick 525b396 [v8.x] deps: V8: cherry-pick 525b396 Dec 14, 2018
@psmarshall psmarshall added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 14, 2018
@Trott
Copy link
Member

Trott commented Dec 14, 2018

Previous CI didn't start because the CERTIFY_SAFE checkbox was left unchecked. (Done it many times myself.)

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

@Trott
Copy link
Member

Trott commented Dec 14, 2018

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19531/

@Trott
Copy link
Member

Trott commented Dec 14, 2018

Resume Build: https://ci.nodejs.org/job/node-test-pull-request/19538/

@Trott
Copy link
Member

Trott commented Dec 14, 2018 •
edited
Loading

Resume Build: https://ci.nodejs.org/job/node-test-pull-request/19543/ ✔️

@Trott
Copy link
Member

Trott commented Dec 14, 2018

Since this is for v8.x-staging, I guess someone in @nodejs/releasers will have to land it.

@targos
Copy link
Member

targos commented Dec 15, 2018

@Trott It should be someone from @nodejs/backporters

@rvagg
Copy link
Member

rvagg commented Dec 18, 2018

Landed in c1611c1

@rvagg rvagg closed this Dec 18, 2018
rvagg pushed a commit that referenced this pull request Dec 18, 2018
@psmarshall @rvagg
deps: cherry-pick 525b396 from V8 upstream
c1611c1 
Original commit message:

    [cpu-profiler] Fix a leak caused by re-logging existing functions.

    Don't re-log all existing functions during StartProcessorIfNotStarted().
    They will already be in the CodeMap attached to the ProfileGenerator and
    re-logging them causes leaks. See the linked bug for more details.

    Bug: v8:8253
    Change-Id: Ibb1a1ab2431c588e8c3a3a9ff714767cdf61a88e
    Reviewed-on: https://chromium-review.googlesource.com/1256763
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#56336}

Refs: v8/v8@525b396

PR-URL: #25041
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Rod Vagg <rod@vagg.org>
MylesBorins pushed a commit that referenced this pull request Dec 18, 2018
@psmarshall @MylesBorins
deps: cherry-pick 525b396 from V8 upstream
ae24aaf 
Original commit message:

    [cpu-profiler] Fix a leak caused by re-logging existing functions.

    Don't re-log all existing functions during StartProcessorIfNotStarted().
    They will already be in the CodeMap attached to the ProfileGenerator and
    re-logging them causes leaks. See the linked bug for more details.

    Bug: v8:8253
    Change-Id: Ibb1a1ab2431c588e8c3a3a9ff714767cdf61a88e
    Reviewed-on: https://chromium-review.googlesource.com/1256763
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#56336}

Refs: v8/v8@525b396

PR-URL: #25041
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Rod Vagg <rod@vagg.org>
MylesBorins pushed a commit that referenced this pull request Dec 26, 2018
@psmarshall @MylesBorins
deps: cherry-pick 525b396 from V8 upstream
8810051 
Original commit message:

    [cpu-profiler] Fix a leak caused by re-logging existing functions.

    Don't re-log all existing functions during StartProcessorIfNotStarted().
    They will already be in the CodeMap attached to the ProfileGenerator and
    re-logging them causes leaks. See the linked bug for more details.

    Bug: v8:8253
    Change-Id: Ibb1a1ab2431c588e8c3a3a9ff714767cdf61a88e
    Reviewed-on: https://chromium-review.googlesource.com/1256763
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#56336}

Refs: v8/v8@525b396

PR-URL: #25041
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Rod Vagg <rod@vagg.org>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@psmarshall @rvagg
deps: cherry-pick 525b396 from V8 upstream
a1aff28 
Original commit message:

    [cpu-profiler] Fix a leak caused by re-logging existing functions.

    Don't re-log all existing functions during StartProcessorIfNotStarted().
    They will already be in the CodeMap attached to the ProfileGenerator and
    re-logging them causes leaks. See the linked bug for more details.

    Bug: v8:8253
    Change-Id: Ibb1a1ab2431c588e8c3a3a9ff714767cdf61a88e
    Reviewed-on: https://chromium-review.googlesource.com/1256763
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#56336}

Refs: v8/v8@525b396

PR-URL: #25041
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Rod Vagg <rod@vagg.org>
@MylesBorins MylesBorins mentioned this pull request Mar 26, 2019
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@rvagg rvagg rvagg approved these changes

@hashseed hashseed hashseed approved these changes

@MylesBorins MylesBorins Awaiting requested review from MylesBorins

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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
@psmarshall @nodejs-github-bot @Trott @targos @rvagg @hashseed