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 missing v8 floating patch #8907

Closed
wants to merge 1 commit into from
Closed

deps: v8 - cherry-pick missing v8 floating patch #8907

wants to merge 1 commit into from

Conversation

mhdawson
Copy link
Member

@mhdawson mhdawson commented Oct 3, 2016 •
edited
Loading

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

v8

Description of change

cherry-pick missing v8 floating patch
2d524bc

Fixes: #8750

Original commit message:

deps: limit regress/regress-crbug-514081 v8 test

regress/regress-crbug-514081 allocates a 2G block of memory
and if there are multiple variants running at the
same time this can lead to crashes, OOM kills or
the OS failing to allocate memory. This patch
limits us to running a single variant of the test

Fixes: #6340
PR-URL: #6678
Reviewed-By: Ben Noorhduis info@bnoordhuis.nl
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Fedor Indutny fedor@indutny.com

@mhdawson
deps: v8 - cherry-pick missing v8 floating patch
1f295f8 
2d524bc

Original commit message:

deps: limit regress/regress-crbug-514081 v8 test

regress/regress-crbug-514081 allocates a 2G block of memory
and if there  are multiple variants running at the
same time this can lead to crashes, OOM kills or
the OS failing to allocate memory.  This patch
limits us to running a single variant of the test

Fixes: nodejs#6340
PR-URL: nodejs#6678
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
@nodejs-github-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Oct 3, 2016
@targos
Copy link
Member

targos commented Oct 3, 2016

LGTM. Sorry for missing it during the V8 update 😕

@mhdawson mhdawson self-assigned this Oct 3, 2016
@mhdawson mhdawson added the test Issues and PRs related to the tests. label Oct 3, 2016
@mhdawson
Copy link
Member Author

mhdawson commented Oct 3, 2016

Earlier v8 run to validate: https://ci.nodejs.org/job/node-test-commit-v8-linux/347/

@mhdawson
Copy link
Member Author

mhdawson commented Oct 3, 2016

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

@MylesBorins
Copy link
Contributor

can we make a document that lists the V8 branches we are maintaining and
floating patches? this could potentially part or the updating V8 doc

/cc @ofrobots

On Mon, Oct 3, 2016, 11:32 AM Michael Dawson notifications@github.com
wrote:

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

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#8907 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecV28mTpQWXDJuZ--Qq8YvZ3ZZOxu_ks5qwSAmgaJpZM4KMtXU
.

bnoordhuis
bnoordhuis approved these changes Oct 3, 2016
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM but can you spell my surname correctly, please? :-)

Also, I'd drop the redundant "v8 -" from the status line.

@mhdawson
Copy link
Member Author

mhdawson commented Oct 3, 2016

CI run looked ok other than Windows failures that were present before/after and not related to this change so I think we are good to go.

@bnoordhuis will address your comments while landing.

@mhdawson
Copy link
Member Author

mhdawson commented Oct 4, 2016

Assume this is non-controversial so will plan to land later today.

mhdawson added a commit that referenced this pull request Oct 5, 2016
@mhdawson
deps: cherry-pick missing v8 floating patch
4b3e4d5 
2d524bc

Original commit message:

  deps: limit regress/regress-crbug-514081 v8 test

  regress/regress-crbug-514081 allocates a 2G block of memory
  and if there  are multiple variants running at the
  same time this can lead to crashes, OOM kills or
  the OS failing to allocate memory.  This patch
  limits us to running a single variant of the test

  Fixes: #6340
  PR-URL: #6678
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Fedor Indutny <fedor@indutny.com>

PR-URL: #8907
Fixes: #8750
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@mhdawson
Copy link
Member Author

mhdawson commented Oct 5, 2016

Landed as 4b3e4d5

@mhdawson mhdawson closed this Oct 5, 2016
jasnell pushed a commit that referenced this pull request Oct 6, 2016
@mhdawson @jasnell
deps: cherry-pick missing v8 floating patch
c0a3ac2 
2d524bc

Original commit message:

  deps: limit regress/regress-crbug-514081 v8 test

  regress/regress-crbug-514081 allocates a 2G block of memory
  and if there  are multiple variants running at the
  same time this can lead to crashes, OOM kills or
  the OS failing to allocate memory.  This patch
  limits us to running a single variant of the test

  Fixes: #6340
  PR-URL: #6678
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Fedor Indutny <fedor@indutny.com>

PR-URL: #8907
Fixes: #8750
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@Fishrock123
Copy link
Contributor

empty patch on v6

@mhdawson mhdawson deleted the v8stress branch March 15, 2017 21:57
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
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

Assignees

@mhdawson mhdawson

Labels
test Issues and PRs related to the tests. 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
@mhdawson @targos @MylesBorins @Fishrock123 @bnoordhuis @nodejs-github-bot