FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

v6.x: deps: backport dfb8d33 from V8 upstream by targos · Pull Request #11483 · nodejs/node · GitHub

/ node Public

v6.x: deps: backport dfb8d33 from V8 upstream - #11483

Closed
targos wants to merge 2 commits into
nodejs:v6.x-stagingfrom
targos:backport-destructuring-fix
ConversationCommits (2)ChecksFiles changed
Closed

v6.x: deps: backport dfb8d33 from V8 upstream#11483
targos wants to merge 2 commits into
nodejs:v6.x-stagingfrom
targos:backport-destructuring-fix

Conversation

targos commented Feb 21, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes: #11480

I added a regression test in a separate commit that can be squashed while landing.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

v8

nodejs-github-bot added v6.x v8 engine Issues and PRs related to the V8 dependency. labels Feb 21, 2017
targos mentioned this pull request Feb 21, 2017
Closed

targos commented Feb 21, 2017

Copy link
Copy Markdown
Member Author

@nodejs/lts @nodejs/v8

richardlau reviewed Feb 22, 2017
View reviewed changes
Comment thread BUILDING.md Outdated

richardlau Feb 22, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

BUILDING.md changes shouldn't be here.

richardlau reviewed Feb 22, 2017
View reviewed changes
Comment thread CONTRIBUTING.md Outdated

richardlau Feb 22, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

CONTRIBUTING.md changes shouldn't be here.

richardlau commented Feb 22, 2017

Copy link
Copy Markdown
Member

@targos I think this is including some unrelated commits.

targos force-pushed the backport-destructuring-fix branch from 655b03b to 304997e Compare February 22, 2017 08:21

targos commented Feb 22, 2017

Copy link
Copy Markdown
Member Author

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

targos commented Feb 28, 2017

Copy link
Copy Markdown
Member Author

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/614/

MylesBorins commented Mar 3, 2017

Copy link
Copy Markdown
Contributor

@targos would you be able to update the commit message of the regression test to follow our guidelines

targos added 2 commits March 3, 2017 13:56
deps: backport dfb8d33 from V8 upstream
b858d69 
Original commit message:
    Reduce the memory footprint of expression classifiers

    This patch attempts to reduce the (stack) memory footprint of
    expression classifiers.  Instead of keeping space in each
    classifier for all possible error messages that will
    (potentially) be reported, if an expression turns out to be
    a pattern or a non-pattern, such error messages are placed in
    a list shared by the FunctionState and each classifier keeps a
    couple of indices in this list.  This requires that classifiers
    are used strictly in a stack-based fashion, which is also in line
    with my previous patch for revisiting non-pattern rewriting.

    R=adamk@chromium.org
    BUG=chromium:528697

    Review-Url: https://codereview.chromium.org/1708193003
    Cr-Commit-Position: refs/heads/master@{nodejs#36897}

Fixes: nodejs#11480
test: add regression test for V8 parse error
6b34806 
Refs: nodejs#11480
targos force-pushed the backport-destructuring-fix branch from 304997e to 6b34806 Compare March 3, 2017 12:59

targos commented Mar 3, 2017

Copy link
Copy Markdown
Member Author

@MylesBorins done

MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
deps: backport dfb8d33 from V8 upstream
b198918 
Original commit message:
    Reduce the memory footprint of expression classifiers

    This patch attempts to reduce the (stack) memory footprint of
    expression classifiers.  Instead of keeping space in each
    classifier for all possible error messages that will
    (potentially) be reported, if an expression turns out to be
    a pattern or a non-pattern, such error messages are placed in
    a list shared by the FunctionState and each classifier keeps a
    couple of indices in this list.  This requires that classifiers
    are used strictly in a stack-based fashion, which is also in line
    with my previous patch for revisiting non-pattern rewriting.

    R=adamk@chromium.org
    BUG=chromium:528697

    Review-Url: https://codereview.chromium.org/1708193003
    Cr-Commit-Position: refs/heads/master@{#36897}

Fixes: #11480
PR-URL: #11483
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
test: add regression test for V8 parse error
b3f32f9 
Refs: #11480
PR-URL: #11483
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>

MylesBorins commented Mar 8, 2017

Copy link
Copy Markdown
Contributor

landed in 1c36e6d...b3f32f9

MylesBorins closed this Mar 8, 2017
MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
deps: backport dfb8d33 from V8 upstream
d0e934f 
Original commit message:
    Reduce the memory footprint of expression classifiers

    This patch attempts to reduce the (stack) memory footprint of
    expression classifiers.  Instead of keeping space in each
    classifier for all possible error messages that will
    (potentially) be reported, if an expression turns out to be
    a pattern or a non-pattern, such error messages are placed in
    a list shared by the FunctionState and each classifier keeps a
    couple of indices in this list.  This requires that classifiers
    are used strictly in a stack-based fashion, which is also in line
    with my previous patch for revisiting non-pattern rewriting.

    R=adamk@chromium.org
    BUG=chromium:528697

    Review-Url: https://codereview.chromium.org/1708193003
    Cr-Commit-Position: refs/heads/master@{#36897}

Fixes: #11480
PR-URL: #11483
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
test: add regression test for V8 parse error
e71fc70 
Refs: #11480
PR-URL: #11483
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
deps: backport dfb8d33 from V8 upstream
3ab070d 
Original commit message:
    Reduce the memory footprint of expression classifiers

    This patch attempts to reduce the (stack) memory footprint of
    expression classifiers.  Instead of keeping space in each
    classifier for all possible error messages that will
    (potentially) be reported, if an expression turns out to be
    a pattern or a non-pattern, such error messages are placed in
    a list shared by the FunctionState and each classifier keeps a
    couple of indices in this list.  This requires that classifiers
    are used strictly in a stack-based fashion, which is also in line
    with my previous patch for revisiting non-pattern rewriting.

    R=adamk@chromium.org
    BUG=chromium:528697

    Review-Url: https://codereview.chromium.org/1708193003
    Cr-Commit-Position: refs/heads/master@{#36897}

Fixes: #11480
PR-URL: #11483
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
test: add regression test for V8 parse error
5365501 
Refs: #11480
PR-URL: #11483
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins mentioned this pull request Mar 9, 2017
Merged
mscdex mentioned this pull request Mar 21, 2017
Closed
targos deleted the backport-destructuring-fix branch June 1, 2017 08:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

richardlau richardlau left review comments
bnoordhuis bnoordhuis approved these changes
jasnell jasnell approved these changes
+1 more reviewer
MylesBorins MylesBorins approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Back | FazBrowse Home | New Git URL