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: cherry-pick 88f8fe1 from upstream V8 #24514

Closed
wants to merge 1 commit into from

Conversation

@hashseed
Copy link
Member

commented Nov 20, 2018

Original commit message:

Fix collection iterator preview with deleted entries

We used to assume that we know the remaining entries returned by the
iterator based on the current index. However, that is not accurate,
since entries skipped by the current index could be deleted.

In the new approach, we allocate conservatively and shrink the result.

R=neis@chromium.org

Bug: v8:8433
Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
Reviewed-on: https://chromium-review.googlesource.com/c/1325966
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57360}

Refs: v8/v8@88f8fe1
Refs: #24053

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • make test-v8 passes
  • commit message follows commit guidelines
@nodejs-github-bot

This comment has been minimized.

Copy link

commented Nov 20, 2018

@hashseed sadly an error occured when I tried to trigger a build :(

@targos

This comment has been minimized.

Copy link
Member

commented Nov 20, 2018

LGTM if you remove deps/v8/src/value-serializer.cc.orig ;)

deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

Refs: v8/v8@88f8fe1

@hashseed hashseed force-pushed the hashseed:backmergepreview branch from 93d6bf1 to 2e27a18 Nov 20, 2018

@hashseed

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2018

LGTM if you remove deps/v8/src/value-serializer.cc.orig ;)

Thanks for spotting this. Fixed :)

@targos
targos approved these changes Nov 20, 2018

@Trott Trott added the author ready label Nov 22, 2018

@addaleax

This comment has been minimized.

Copy link
Member

commented Nov 23, 2018

@danbev

This comment has been minimized.

Copy link
Member

commented Nov 23, 2018

Re-run of failing node-test-commit-windows-fanned.

@targos

This comment has been minimized.

Copy link
Member

commented Nov 24, 2018

Landed in 70e2f9d

@targos targos closed this Nov 24, 2018

targos added a commit that referenced this pull request Nov 24, 2018
deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

Refs: v8/v8@88f8fe1

PR-URL: #24514
Refs: #24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit that referenced this pull request Nov 24, 2018
deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

Refs: v8/v8@88f8fe1

PR-URL: #24514
Refs: #24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
rvagg added a commit that referenced this pull request Nov 28, 2018
deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

Refs: v8/v8@88f8fe1

PR-URL: #24514
Refs: #24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit to targos/node that referenced this pull request Dec 4, 2018
deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

Refs: v8/v8@88f8fe1

PR-URL: nodejs#24514
Refs: nodejs#24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@BridgeAR BridgeAR referenced this pull request Dec 5, 2018
4 of 4 tasks complete
pull bot pushed a commit to shakir-abdo/node that referenced this pull request Dec 6, 2018
deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

Refs: v8/v8@88f8fe1

PR-URL: nodejs#24514
Refs: nodejs#24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
refack added a commit to refack/node that referenced this pull request Jan 14, 2019
deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

Refs: v8/v8@88f8fe1

PR-URL: nodejs#24514
Refs: nodejs#24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
refack added a commit to refack/node that referenced this pull request Jan 14, 2019
deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

Refs: v8/v8@88f8fe1

PR-URL: nodejs#24514
Refs: nodejs#24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

@hashseed hashseed deleted the hashseed:backmergepreview branch Jan 28, 2019

@BethGriggs

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

This change does not land cleanly on v10.x-staging. I've added the backport-requested-v10.x label, but feel free to swap to dont-land-on- if this change shouldn't land on v10.x.

addaleax added a commit to addaleax/node that referenced this pull request May 26, 2019
deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

[The backport to v10.x resolves merge conflicts due to a different way
of accessing the “hole” value in V8, different signatures of the
`Handle` constructor and the `Shrink()` method, and neighbouring-line
conflicts in the test file.]

Refs: v8/v8@88f8fe1
Fixes: nodejs#27882
PR-URL: nodejs#24514
Refs: nodejs#24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@addaleax addaleax referenced this pull request May 26, 2019
4 of 4 tasks complete
BethGriggs added a commit that referenced this pull request Jun 6, 2019
deps: cherry-pick 88f8fe1 from upstream V8
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

[The backport to v10.x resolves merge conflicts due to a different way
of accessing the “hole” value in V8, different signatures of the
`Handle` constructor and the `Shrink()` method, and neighbouring-line
conflicts in the test file.]

Refs: v8/v8@88f8fe1
Fixes: #27882

Backport-PR-URL: #27894
PR-URL: #24514
Refs: #24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@BethGriggs BethGriggs referenced this pull request Jul 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.