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.

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

deps: V8: backport f7771e5b0cc4 #31957

Closed
wants to merge 1 commit into from

Conversation

mmarchini
Copy link
Contributor

This fixes a performance regression on V8 where frequent delete/reinsertions on a POJO object can lead to a bit overflow, causing subsequent property insertions to take exponentially more time.

We should try to backport this to v12, since the regression started there (yes, I'm volunteering to backport if it doesn't land cleanly).


Original commit message:

[runtime] Recompute enumeration indices of dictionaries upon bitfield overflow

Otherwise we'll get weird semantics when enumerating objects after many
deletes/reinserts.

Bug: chromium:1033771
Change-Id: If0a459169c3794a30d9632d09e80da3cfcd4302c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993966
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65690}

Refs: v8/v8@f7771e5

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Original commit message:

    [runtime] Recompute enumeration indices of dictionaries upon bitfield overflow

    Otherwise we'll get weird semantics when enumerating objects after many
    deletes/reinserts.

    Bug: chromium:1033771
    Change-Id: If0a459169c3794a30d9632d09e80da3cfcd4302c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993966
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65690}

Refs: v8/v8@f7771e5
@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 Feb 26, 2020
@mmarchini mmarchini added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 26, 2020
Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

V8 CI is failing on two platforms but those are known failures. Re ran linux CI which was having an issue but this should be ready to land

@MylesBorins
Copy link
Contributor

BTW this patch lands cleanly on both master and 13.x but will require a manual backport to 12.x

mmarchini added a commit that referenced this pull request Feb 28, 2020
Original commit message:

    [runtime] Recompute enumeration indices of dictionaries upon bitfield overflow

    Otherwise we'll get weird semantics when enumerating objects after many
    deletes/reinserts.

    Bug: chromium:1033771
    Change-Id: If0a459169c3794a30d9632d09e80da3cfcd4302c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993966
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65690}

Refs: v8/v8@f7771e5

PR-URL: #31957
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@mmarchini
Copy link
Contributor Author

Landed in 3d894d0

@codebytere
Copy link
Member

@MylesBorins i just tried and it seems to conflict on v13 🤔

node on git:v13.x-staging ❯ g cp 3d894d0511                                3:09PM
error: could not apply 3d894d0511... deps: V8: backport f7771e5b0cc4
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
Recorded preimage for 'common.gypi'

Not sure what the embedder string there should be, that's the conflict source.

@MylesBorins
Copy link
Contributor

MylesBorins commented Mar 1, 2020 via email

codebytere pushed a commit that referenced this pull request Mar 1, 2020
Original commit message:

    [runtime] Recompute enumeration indices of dictionaries upon bitfield overflow

    Otherwise we'll get weird semantics when enumerating objects after many
    deletes/reinserts.

    Bug: chromium:1033771
    Change-Id: If0a459169c3794a30d9632d09e80da3cfcd4302c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993966
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65690}

Refs: v8/v8@f7771e5

PR-URL: #31957
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@codebytere codebytere mentioned this pull request Mar 1, 2020
mmarchini added a commit to mmarchini/node that referenced this pull request Mar 2, 2020
Original commit message:

    [runtime] Recompute enumeration indices of dictionaries upon bitfield overflow

    Otherwise we'll get weird semantics when enumerating objects after many
    deletes/reinserts.

    Bug: chromium:1033771
    Change-Id: If0a459169c3794a30d9632d09e80da3cfcd4302c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993966
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65690}

Refs: v8/v8@f7771e5

PR-URL: nodejs#31957
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
Original commit message:

    [runtime] Recompute enumeration indices of dictionaries upon bitfield overflow

    Otherwise we'll get weird semantics when enumerating objects after many
    deletes/reinserts.

    Bug: chromium:1033771
    Change-Id: If0a459169c3794a30d9632d09e80da3cfcd4302c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993966
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65690}

Refs: v8/v8@f7771e5

PR-URL: #31957
Backport-PR-URL: #32053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@codebytere codebytere mentioned this pull request Mar 17, 2020
codebytere pushed a commit that referenced this pull request Mar 23, 2020
Original commit message:

    [runtime] Recompute enumeration indices of dictionaries upon bitfield overflow

    Otherwise we'll get weird semantics when enumerating objects after many
    deletes/reinserts.

    Bug: chromium:1033771
    Change-Id: If0a459169c3794a30d9632d09e80da3cfcd4302c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993966
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65690}

Refs: v8/v8@f7771e5

PR-URL: #31957
Backport-PR-URL: #32053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 30, 2020
Original commit message:

    [runtime] Recompute enumeration indices of dictionaries upon bitfield overflow

    Otherwise we'll get weird semantics when enumerating objects after many
    deletes/reinserts.

    Bug: chromium:1033771
    Change-Id: If0a459169c3794a30d9632d09e80da3cfcd4302c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993966
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65690}

Refs: v8/v8@f7771e5

PR-URL: #31957
Backport-PR-URL: #32053
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

7 participants