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: V8: cherry-pick d3a1a5b6c491 #31005

Closed
wants to merge 1 commit into from
Closed

deps: V8: cherry-pick d3a1a5b6c491 #31005

wants to merge 1 commit into from

Conversation

targos
Copy link
Member

@targos targos commented Dec 17, 2019

Original commit message:

[objects] Fix memory leak in PrototypeUsers::Add

PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
before growing the array. Not reusing empty slots caused a memory leak.

It might also be desirable to shrink the WeakArrayList in the future.
Right now it is only compacted when invoking CreateBlob.

Also removed unused PrototypeUsers::IsEmptySlot declaration.

Bug: v8:10031
Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: #30753

@targos
deps: V8: cherry-pick d3a1a5b6c491
cceb2a8 
Original commit message:

    [objects] Fix memory leak in PrototypeUsers::Add

    PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
    before growing the array. Not reusing empty slots caused a memory leak.

    It might also be desirable to shrink the WeakArrayList in the future.
    Right now it is only compacted when invoking CreateBlob.

    Also removed unused PrototypeUsers::IsEmptySlot declaration.

    Bug: v8:10031
    Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
    Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: nodejs#30753
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Dec 17, 2019
@targos targos mentioned this pull request Dec 17, 2019
Closed
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 17, 2019 •
edited by targos
Loading

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

@targos targos added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 18, 2019
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 21, 2019 •
edited by Trott
Loading

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

targos added a commit that referenced this pull request Dec 23, 2019
@targos
deps: V8: cherry-pick d3a1a5b6c491
05041d3 
Original commit message:

    [objects] Fix memory leak in PrototypeUsers::Add

    PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
    before growing the array. Not reusing empty slots caused a memory leak.

    It might also be desirable to shrink the WeakArrayList in the future.
    Right now it is only compacted when invoking CreateBlob.

    Also removed unused PrototypeUsers::IsEmptySlot declaration.

    Bug: v8:10031
    Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
    Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: #30753

PR-URL: #31005
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos
Copy link
Member Author

targos commented Dec 23, 2019

Landed in 05041d3

@targos targos closed this Dec 23, 2019
@targos targos deleted the fix-30753 branch December 23, 2019 16:50
@assaf-xm
Copy link

Will it be merged to v12.14.1-proposal?

BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
@targos @BridgeAR
deps: V8: cherry-pick d3a1a5b6c491
7e13ae7 
Original commit message:

    [objects] Fix memory leak in PrototypeUsers::Add

    PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
    before growing the array. Not reusing empty slots caused a memory leak.

    It might also be desirable to shrink the WeakArrayList in the future.
    Right now it is only compacted when invoking CreateBlob.

    Also removed unused PrototypeUsers::IsEmptySlot declaration.

    Bug: v8:10031
    Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
    Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: #30753

PR-URL: #31005
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
Merged
@assaf-xm
Copy link

assaf-xm commented Jan 9, 2020

@BridgeAR , which version of node12 will include this fix?

@BridgeAR
Copy link
Member

BridgeAR commented Jan 9, 2020

@assaf-xm it is likely that it's included the next upcoming v12.x release.

@deepak1556 deepak1556 mentioned this pull request Jan 13, 2020
Merged
6 tasks
targos added a commit that referenced this pull request Jan 14, 2020
@targos
deps: V8: cherry-pick d3a1a5b6c491
ccf1178 
Original commit message:

    [objects] Fix memory leak in PrototypeUsers::Add

    PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
    before growing the array. Not reusing empty slots caused a memory leak.

    It might also be desirable to shrink the WeakArrayList in the future.
    Right now it is only compacted when invoking CreateBlob.

    Also removed unused PrototypeUsers::IsEmptySlot declaration.

    Bug: v8:10031
    Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
    Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: #30753

PR-URL: #31005
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos targos mentioned this pull request Jan 15, 2020
Closed
This was referenced Jan 23, 2020
Closed
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
@targos @BethGriggs
deps: V8: cherry-pick d3a1a5b6c491
c004cf5 
Original commit message:

    [objects] Fix memory leak in PrototypeUsers::Add

    PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
    before growing the array. Not reusing empty slots caused a memory leak.

    It might also be desirable to shrink the WeakArrayList in the future.
    Right now it is only compacted when invoking CreateBlob.

    Also removed unused PrototypeUsers::IsEmptySlot declaration.

    Bug: v8:10031
    Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
    Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: #30753

PR-URL: #31005
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
Merged
@snyk-bot snyk-bot mentioned this pull request Jun 1, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@Trott Trott Trott approved these changes

@cjihrig cjihrig cjihrig approved these changes

@richardlau richardlau richardlau approved these changes

@devsnek devsnek devsnek 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. 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.

Generator functions - memory leak
10 participants
@targos @nodejs-github-bot @assaf-xm @BridgeAR @bnoordhuis @Trott @cjihrig @richardlau @devsnek @MylesBorins