GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — deps: cherry-pick b93c80a from v8 upstream by matthewloring · Pull Request #7689 · nodejs/node

Conversation

@matthewloring

@matthewloring matthewloring commented Jul 12, 2016

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

deps

Description of change

Original commit message:

If we can't rehash the backing store for weak sets & maps, do a last
resort GC

BUG=v8:4909
R=[email protected]

Committed: https://crrev.com/b93c80a6039c757723e70420ae73375b5d277814
Cr-Commit-Position: refs/heads/master@{#37591}

Fixes: #6180

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nodejs-github-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Jul 12, 2016
@matthewloring

matthewloring commented Jul 12, 2016

Copy link
Copy Markdown
Author

Resolution of #6398.
\cc @nodejs/v8
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ofrobots ofrobots added the lts-watch-v4.x label Jul 12, 2016
@ofrobots

ofrobots commented Jul 12, 2016

Copy link
Copy Markdown
Contributor

/cc @nodejs/v8.

  • Added lts-watch-4.x label. v4.x would also need the fixes in deps: backport e093a04, 09db540 from upstream V8 #6398, but are not needed on master.
  • @matthewloring: for V8 5.2 and 5.3 can you request a merge in the upstream repository.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins

MylesBorins commented Jul 12, 2016

Copy link
Copy Markdown
Contributor

@ofrobots would you be willing to put together a PR against v4.x-staging that includes all of those changes?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ofrobots

ofrobots commented Jul 12, 2016

Copy link
Copy Markdown
Contributor

@thealphanerd Sure. Once this has baked for a couple of weeks on master and v6.x?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins

MylesBorins commented Jul 12, 2016

Copy link
Copy Markdown
Contributor

sounds reasonable to me
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bnoordhuis
bnoordhuis reviewed Jul 13, 2016
View reviewed changes
Comment thread deps/v8/src/objects.cc Outdated

@bnoordhuis bnoordhuis Jul 13, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The call to HasSufficientCapacity() with n == NumberOfElements() + 1 doesn't look obviously correct to me. Here is its implementation:

template <typename Derived, typename Shape, typename Key>
bool HashTable<Derived, Shape, Key>::HasSufficientCapacity(int n) {
  int capacity = Capacity();
  int nof = NumberOfElements() + n;
  int nod = NumberOfDeletedElements();
  // Return true if:
  //   50% is still free after adding n elements and
  //   at most 50% of the free elements are deleted elements.
  if (nod <= (capacity - nof) >> 1) {
    int needed_free = nof >> 1;
    if (nof + needed_free <= capacity) return true;
  }
  return false;
}

capacity - nof can be < 0 if capacity < NumberOfElements() * 2 + 1 and right-shifting a negative number has implementation dependent behavior. I think compilers all implement it as an arithmetic shift but I'd say it's better to avoid it altogether.

As well, because it uses int, there is a potential for integer overflow when NumberOfElements() is large.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@jeisinger jeisinger Jul 19, 2016

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The maximum size of the backing store is 128MB so that this can never happen.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bnoordhuis bnoordhuis Jul 20, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

You mean integer overflow? What about the capacity < NumberOfElements() * 2 + 1 case?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@jeisinger jeisinger Jul 20, 2016

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

JSWeakCollection uses an ObjectHashTable as backing store. ObjectHashTableShape::kEntrySize is 2, ObjectHashTable::kMaxCapacity (FixedArray::kMaxSize(128 MB * kPointerSize) - FixArray::kHeaderSize) / kPointerSize / kEntrySize which is a bit less than 64MB, and NumberOfElements() * 2 + 1 can be at most 134217729 which fits nicely into a signed 32bit integer.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bnoordhuis bnoordhuis Jul 20, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry, I understand that the integer overflow case is covered. I mean the right shift when capacity < nof.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@jeisinger jeisinger Jul 20, 2016

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ah, sorry, I didn't read carefully enough what you actually wrote. https://codereview.chromium.org/2162333002 should address this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@matthewloring matthewloring Jul 20, 2016

Copy link
Copy Markdown
Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I can bring in 2162333002 as part of this PR. Does the fix there look good to you @bnoordhuis?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bnoordhuis bnoordhuis Jul 20, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good at a quick glance.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@matthewloring matthewloring Jul 20, 2016

Copy link
Copy Markdown
Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Uploaded.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@matthewloring

matthewloring commented Jul 13, 2016

Copy link
Copy Markdown
Author

/cc @jeisinger who did the original PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnoordhuis

bnoordhuis commented Jul 22, 2016

Copy link
Copy Markdown
Member

LGTM
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matthewloring

matthewloring commented Jul 22, 2016

Copy link
Copy Markdown
Author

CI: https://ci.nodejs.org/job/node-test-pull-request/3390/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnoordhuis

bnoordhuis commented Jul 23, 2016

Copy link
Copy Markdown
Member

Can you also run the V8 test suite? That's the https://ci.nodejs.org/job/node-test-commit-v8-linux/ job.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ofrobots

ofrobots commented Jul 23, 2016

Copy link
Copy Markdown
Contributor

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/231/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matthewloring

matthewloring commented Jul 25, 2016

Copy link
Copy Markdown
Author

@mhdawson @joransiu Any thoughts on the s390x failures?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@joransiu

joransiu commented Jul 25, 2016

Copy link
Copy Markdown
Contributor

@matthewloring The s390x failures in StackAlignment in test-platform.cc (https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel72-s390x,v8test=v8test/231/console) should be fixed via #7771, which should have landed already.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matthewloring

matthewloring commented Jul 25, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Author

Running after rebase.
CI: https://ci.nodejs.org/job/node-test-pull-request/3407/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/238/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott

Trott commented Jul 25, 2016

Copy link
Copy Markdown
Member

Will probably want to re-run CI after #7873 lands (hopefully within the hour). That will fix the bad build on four of the Linux hosts.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Matt Loring added 2 commits July 25, 2016 14:19
Matt Loring
deps: cherry-pick b93c80a from v8 upstream
271a498 
Original commit message:

If we can't rehash the backing store for weak sets & maps, do a last
resort GC

BUG=v8:4909
[email protected]

Committed: https://crrev.com/b93c80a6039c757723e70420ae73375b5d277814
Cr-Commit-Position: refs/heads/master@{#37591}

Fixes: #6180
Matt Loring
deps: cherry-pick a76d133 from v8 upstream
965ff41 
Original commit message:

Fix incorrect parameter to HasSufficientCapacity

It takes the number of additional elements, not the total target
capacity.

Also, avoid right-shifting a negative integer as this is undefined in
general

BUG=v8:4909
[email protected]

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

matthewloring commented Jul 26, 2016

Copy link
Copy Markdown
Author

Only the windows CI is failing and it seems to be failing similarly in other builds. I'll land this tomorrow if nobody objects.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott

Trott commented Jul 26, 2016

Copy link
Copy Markdown
Member

Looks like the Windows issue has been fixed. Fingers-crossed re-run of CI: https://ci.nodejs.org/job/node-test-pull-request/3415/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matthewloring

matthewloring commented Jul 26, 2016

Copy link
Copy Markdown
Author

Landed in a3d62bd, e22ffef.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matthewloring matthewloring closed this Jul 26, 2016
@matthewloring
matthewloring deleted the v8-4909 branch July 26, 2016 17:47
@ofrobots

ofrobots commented Jul 26, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

@matthewloring do you also want to take care of this:

@thealphanerd

would you be willing to put together a PR against v4.x-staging that includes all of those changes?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matthewloring
matthewloring restored the v8-4909 branch July 26, 2016 22:14
@matthewloring
matthewloring deleted the v8-4909 branch July 26, 2016 22:35
@matthewloring

matthewloring commented Jul 26, 2016

Copy link
Copy Markdown
Author

@ofrobots @thealphanerd Opened here: #7883.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ofrobots pushed a commit to ofrobots/node that referenced this pull request Aug 25, 2016
Matt Loring @ofrobots
deps: cherry-pick b93c80a from v8 upstream
3b2f692 
Original commit message:

If we can't rehash the backing store for weak sets & maps, do a last
resort GC

BUG=v8:4909
[email protected]

Committed: https://crrev.com/b93c80a6039c757723e70420ae73375b5d277814
Cr-Commit-Position: refs/heads/master@{#37591}

Fixes: nodejs#6180
PR-URL: nodejs#7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
ofrobots pushed a commit to ofrobots/node that referenced this pull request Aug 25, 2016
Matt Loring @ofrobots
deps: cherry-pick a76d133 from v8 upstream
4a20869 
Original commit message:

Fix incorrect parameter to HasSufficientCapacity

It takes the number of additional elements, not the total target
capacity.

Also, avoid right-shifting a negative integer as this is undefined in
general

BUG=v8:4909
[email protected]

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

Fixes: nodejs#6180
PR-URL: nodejs#7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@evanlucas evanlucas mentioned this pull request Aug 26, 2016
2016-08-26, Version 6.5.0 (Current) #8253
Merged
@MylesBorins MylesBorins added dont-land-on-v4.x and removed lts-watch-v4.x labels Oct 10, 2016
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
@ofrobots Myles Borins
deps: backport e093a04, 09db540 from upstream V8
5ab6edd 
Original commit messages:
v8/v8@e093a04
  Rehash and clear deleted entries in weak collections during GC
  Otherwise, they'll just keep growing until we run out of memory or hit the FixedArray's maximum capacity.

  BUG=v8:4909
  [email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1877233005

  Cr-Commit-Position: refs/heads/master@{#35514}

v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC
  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{#35538}

Ref: https://crbug.com/v8/4909
Ref: #7883
Fixes: #6180
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Matt Loring Myles Borins
deps: cherry-pick b93c80a from v8 upstream
d664fbd 
Original commit message:

If we can't rehash the backing store for weak sets & maps, do a last
resort GC

BUG=v8:4909
[email protected]

Committed: https://crrev.com/b93c80a6039c757723e70420ae73375b5d277814
Cr-Commit-Position: refs/heads/master@{#37591}

Fixes: #6180
Ref: #7883
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Matt Loring Myles Borins
deps: backport a76d133 from v8 upstream
32b2fb8 
Original commit message:

  Fix incorrect parameter to HasSufficientCapacity

  It takes the number of additional elements, not the total target
  capacity.

  Also, avoid right-shifting a negative integer as this is undefined in
  general

  BUG=v8:4909
  [email protected]

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

Fixes: #6180
Ref: #7883
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
@ofrobots Myles Borins
deps: backport e093a04, 09db540 from upstream V8
6b191f3 
Original commit messages:
v8/v8@e093a04
  Rehash and clear deleted entries in weak collections during GC
  Otherwise, they'll just keep growing until we run out of memory or hit the FixedArray's maximum capacity.

  BUG=v8:4909
  [email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1877233005

  Cr-Commit-Position: refs/heads/master@{#35514}

v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC
  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{#35538}

Ref: https://crbug.com/v8/4909
Ref: #7883
Fixes: #6180
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Matt Loring Myles Borins
deps: cherry-pick b93c80a from v8 upstream
2cfec2d 
Original commit message:

If we can't rehash the backing store for weak sets & maps, do a last
resort GC

BUG=v8:4909
[email protected]

Committed: https://crrev.com/b93c80a6039c757723e70420ae73375b5d277814
Cr-Commit-Position: refs/heads/master@{#37591}

Fixes: #6180
Ref: #7883
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Matt Loring Myles Borins
deps: backport a76d133 from v8 upstream
b6692ba 
Original commit message:

  Fix incorrect parameter to HasSufficientCapacity

  It takes the number of additional elements, not the total target
  capacity.

  Also, avoid right-shifting a negative integer as this is undefined in
  general

  BUG=v8:4909
  [email protected]

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

Fixes: #6180
Ref: #7883
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
@ofrobots @rvagg
deps: backport e093a04, 09db540 from upstream V8
74d37d0 
Original commit messages:
v8/v8@e093a04
  Rehash and clear deleted entries in weak collections during GC
  Otherwise, they'll just keep growing until we run out of memory or hit the FixedArray's maximum capacity.

  BUG=v8:4909
  [email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1877233005

  Cr-Commit-Position: refs/heads/master@{#35514}

v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC
  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{#35538}

Ref: https://crbug.com/v8/4909
Ref: #7883
Fixes: #6180
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
Matt Loring @rvagg
deps: cherry-pick b93c80a from v8 upstream
492baa9 
Original commit message:

If we can't rehash the backing store for weak sets & maps, do a last
resort GC

BUG=v8:4909
[email protected]

Committed: https://crrev.com/b93c80a6039c757723e70420ae73375b5d277814
Cr-Commit-Position: refs/heads/master@{#37591}

Fixes: #6180
Ref: #7883
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
Matt Loring @rvagg
deps: backport a76d133 from v8 upstream
93e5381 
Original commit message:

  Fix incorrect parameter to HasSufficientCapacity

  It takes the number of additional elements, not the total target
  capacity.

  Also, avoid right-shifting a negative integer as this is undefined in
  general

  BUG=v8:4909
  [email protected]

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

Fixes: #6180
Ref: #7883
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
@ofrobots @MylesBorins
deps: backport e093a04, 09db540 from upstream V8
2d07fd7 
Original commit messages:
v8/v8@e093a04
  Rehash and clear deleted entries in weak collections during GC
  Otherwise, they'll just keep growing until we run out of memory or hit the FixedArray's maximum capacity.

  BUG=v8:4909
  [email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1877233005

  Cr-Commit-Position: refs/heads/master@{#35514}

v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC
  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{#35538}

Ref: https://crbug.com/v8/4909
Ref: #7883
Fixes: #6180
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
Matt Loring @MylesBorins
deps: cherry-pick b93c80a from v8 upstream
e1f12fb 
Original commit message:

If we can't rehash the backing store for weak sets & maps, do a last
resort GC

BUG=v8:4909
[email protected]

Committed: https://crrev.com/b93c80a6039c757723e70420ae73375b5d277814
Cr-Commit-Position: refs/heads/master@{#37591}

Fixes: #6180
Ref: #7883
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
Matt Loring @MylesBorins
deps: backport a76d133 from v8 upstream
2b01bc8 
Original commit message:

  Fix incorrect parameter to HasSufficientCapacity

  It takes the number of additional elements, not the total target
  capacity.

  Also, avoid right-shifting a negative integer as this is undefined in
  general

  BUG=v8:4909
  [email protected]

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

Fixes: #6180
Ref: #7883
PR-URL: #7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
V4.6.2 proposal #9298
Closed
BethGriggs pushed a commit to ibmruntimes/node that referenced this pull request Nov 9, 2016
@ofrobots @BethGriggs
deps: backport e093a04, 09db540 from upstream V8
b03629e 
Original commit messages:
v8/v8@e093a04
  Rehash and clear deleted entries in weak collections during GC
  Otherwise, they'll just keep growing until we run out of memory or hit the FixedArray's maximum capacity.

  BUG=v8:4909
  [email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1877233005

  Cr-Commit-Position: refs/heads/master@{#35514}

v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC
  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{#35538}

Ref: https://crbug.com/v8/4909
Ref: nodejs/node#7883
Fixes: nodejs/node#6180
PR-URL: nodejs/node#7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
BethGriggs pushed a commit to ibmruntimes/node that referenced this pull request Nov 9, 2016
Matt Loring @BethGriggs
deps: cherry-pick b93c80a from v8 upstream
af2da0f 
Original commit message:

If we can't rehash the backing store for weak sets & maps, do a last
resort GC

BUG=v8:4909
[email protected]

Committed: https://crrev.com/b93c80a6039c757723e70420ae73375b5d277814
Cr-Commit-Position: refs/heads/master@{#37591}

Fixes: nodejs/node#6180
Ref: nodejs/node#7883
PR-URL: nodejs/node#7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
BethGriggs pushed a commit to ibmruntimes/node that referenced this pull request Nov 9, 2016
Matt Loring @BethGriggs
deps: backport a76d133 from v8 upstream
20aef57 
Original commit message:

  Fix incorrect parameter to HasSufficientCapacity

  It takes the number of additional elements, not the total target
  capacity.

  Also, avoid right-shifting a negative integer as this is undefined in
  general

  BUG=v8:4909
  [email protected]

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

Fixes: nodejs/node#6180
Ref: nodejs/node#7883
PR-URL: nodejs/node#7689
Reviewed-By: Matt Loring <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@bnoordhuis bnoordhuis mentioned this pull request Jul 21, 2017
SEGFAULT in node 4.8.4 on linux #14228
Closed
abernix added a commit to abernix/node that referenced this pull request Aug 14, 2017
@abernix
deps: cherry-pick 09db540,686558d from V8 upstream
5f18fb2 
Original commit messages:
v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC

  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{nodejs#35538}

v8/v8@686558d
  Fix comment about when we rehash ObjectHashTables before growing them

  [email protected]
  BUG=

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

Refs: https://crbug.com/v8/4909
Refs: nodejs#6180
Refs: nodejs#7689
Refs: nodejs#6398
Fixes: nodejs#14228
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
@abernix @MylesBorins
deps: cherry-pick 09db540,686558d from V8 upstream
5d7f68a 
Original commit messages:
v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC

  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{#35538}

v8/v8@686558d
  Fix comment about when we rehash ObjectHashTables before growing them

  [email protected]
  BUG=

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

Refs: https://crbug.com/v8/4909
Refs: #6180
Refs: #7689
Refs: #6398
Fixes: #14228

PR-URL: #14829
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
@abernix @MylesBorins
deps: cherry-pick 09db540,686558d from V8 upstream
71f9cdf 
Original commit messages:
v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC

  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{#35538}

v8/v8@686558d
  Fix comment about when we rehash ObjectHashTables before growing them

  [email protected]
  BUG=

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

Refs: https://crbug.com/v8/4909
Refs: #6180
Refs: #7689
Refs: #6398
Fixes: #14228

PR-URL: #14829
Reviewed-By: Ben Noordhuis <[email protected]>
gibfahn pushed a commit to ibmruntimes/node that referenced this pull request Nov 24, 2017
@abernix @gibfahn
deps: cherry-pick 09db540,686558d from V8 upstream
b77def2 
Original commit messages:
v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC

  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{#35538}

v8/v8@686558d
  Fix comment about when we rehash ObjectHashTables before growing them

  [email protected]
  BUG=

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

Refs: https://crbug.com/v8/4909
Refs: nodejs/node#6180
Refs: nodejs/node#7689
Refs: nodejs/node#6398
Fixes: nodejs/node#14228

PR-URL: nodejs/node#14829
Reviewed-By: Ben Noordhuis <[email protected]>
@hackerkid hackerkid mentioned this pull request Jan 2, 2018
Request : create a Node.js community on Slack or Discord #17821
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

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.

Memory leak in simple test case involving WeakSet

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@matthewloring @ofrobots @MylesBorins @bnoordhuis @joransiu @Trott @evanlucas @jeisinger @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.