deps: backport e093a04, 09db540 from upstream V8#6398

ofrobots · 2016-04-26T13:52:47Z

Checklist

tests and code linting passes
the commit message follows commit guidelines

Affected core subsystem(s)

deps

Description of change

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
R=[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
R=[email protected],[email protected]
LOG=n
Review URL: https://codereview.chromium.org/1890123002
Cr-Commit-Position: refs/heads/master@{#35538}

V8-Bug: https://crbug.com/v8/4909
Fixes: #6180
Ref: #6375
R=@nodejs/v8
CI: https://ci.nodejs.org/job/node-test-pull-request/2399/

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@{nodejs#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@{nodejs#35538} V8-Bug: https://crbug.com/v8/4909Fixes: nodejs#6180

indutny · 2016-04-26T15:13:30Z

Rubber-stamp LGTM

bnoordhuis · 2016-04-26T15:35:05Z

deps/v8/src/heap/mark-compact.cc

 table->RemoveEntry(i);
 }
 }
+// Rehash if more than 25% of the entries are deleted entries.


More than 50%, I think?

actually, it's 1/3, but in any case the comment is wrong

bnoordhuis · 2016-04-26T15:39:43Z

LGTM

ChALkeR · 2016-04-26T15:40:01Z

Does this solve the testcase at #6375 (comment)?
I am not able to test that atm.
Note: this is just a question, it should not hold this PR.

jasnell · 2016-04-26T15:43:43Z

Should this go into v6?

ChALkeR · 2016-04-26T15:50:58Z

@jasnell I think that's the reason for backporting, isn't it?

ofrobots · 2016-04-26T16:00:13Z

This can go in a patch level, no need to hold the release for this.
On Apr 26, 2016 8:52 AM, "Сковорода Никита Андреевич" <
[email protected]> wrote:

@jasnell https://github.com/jasnell I think that's the reason for
backporting, isn't it?
—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#6398 (comment)

jasnell · 2016-04-26T16:02:02Z

Ok. LGTM btw

jeisinger · 2016-04-27T07:55:56Z

deps/v8/src/heap/mark-compact.cc

+if ((table->NumberOfDeletedElements() << kJSWeakCollectionLoadFactorExp) >
+ table->NumberOfElements()){
+ HandleScope scope(heap()->isolate());
+ table->Rehash(heap()->isolate()->factory()->undefined_value());


@ofrobots this is the wrong CL, you should only merge the "Reland" CL.

@jeisinger: Applying the two patches from the 'Reland CL' (https://codereview.chromium.org/1890123002/#ps20001) produces the same patch as 3e8d7a7. Maybe I am missing something?
/cc @matthewloring re: #7883

ChALkeR · 2016-04-27T17:01:24Z

Btw, @jeisinger, @natorion, what about back-porting this to v8 5.0.x?

natorion · 2016-04-28T14:04:08Z

Could you please use the process outlined in https://github.com/v8/v8/wiki/Merging%20&%20Patching ?

ofrobots · 2016-05-04T17:12:22Z

Superceded by #6572. I will open a separate PR to backport the relevant commits to v4.x.

ofrobots · 2016-05-10T13:13:44Z

Upon verification, the upstream fixes are not sufficient for solving the original issue in #6180. Closing this PR. Let's wait until https://crbug.com/v8/4909 gets fully resolved.

See Build nodejs#6398.

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#6398Fixes: nodejs#14228

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: #6398Fixes: #14228 PR-URL: #14829 Reviewed-By: Ben Noordhuis <[email protected]>

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#6398Fixes: nodejs/node#14228 PR-URL: nodejs/node#14829 Reviewed-By: Ben Noordhuis <[email protected]>

ofrobots added 2 commits April 26, 2016 06:44

[SQUASH] fixup needed for backport
5b72b47

ofrobots added v8 engine Issues and PRs related to the V8 dependency. lts-watch-v4.x labels Apr 26, 2016

Fishrock123 modified the milestone: 6.0.0 Apr 26, 2016

bnoordhuis reviewed Apr 26, 2016
View reviewed changes

ChALkeR added the memory Issues and PRs related to the memory management or memory footprint. label Apr 26, 2016

jeisinger reviewed Apr 27, 2016
View reviewed changes

ofrobots closed this May 10, 2016

ChALkeR mentioned this pull request May 10, 2016
Memory leaks in loops with Promise #6673
Closed

MylesBorins removed the lts-watch-v4.x label May 18, 2016

matthewloring mentioned this pull request Jul 12, 2016
deps: cherry-pick b93c80a from v8 upstream #7689
Closed
2 tasks

matthewloring mentioned this pull request Jul 27, 2016
deps: cherry-pick b93c80a from v8 upstream #7883
Closed
2 tasks

fhinkel added a commit to v8/node that referenced this pull request Jun 28, 2017
[HACK] test: deactivate failing tests
d9ffec7
See Build nodejs#6398.

ofrobots deleted the fix-6180-master branch July 21, 2017 14:27

Uh oh!

deps: backport e093a04, 09db540 from upstream V8#6398

deps: backport e093a04, 09db540 from upstream V8 #6398

Uh oh!

Conversation

ofrobots commented Apr 26, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

indutny commented Apr 26, 2016

Uh oh!

bnoordhuisApr 26, 2016

Choose a reason for hiding this comment

Uh oh!

jeisingerApr 27, 2016

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented Apr 26, 2016

Uh oh!

ChALkeR commented Apr 26, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell commented Apr 26, 2016

Uh oh!

ChALkeR commented Apr 26, 2016

Uh oh!

ofrobots commented Apr 26, 2016

Uh oh!

jasnell commented Apr 26, 2016

Uh oh!

jeisingerApr 27, 2016

Choose a reason for hiding this comment

Uh oh!

ofrobotsAug 1, 2016

Choose a reason for hiding this comment

Uh oh!

ChALkeR commented Apr 27, 2016

Uh oh!

natorion commented Apr 28, 2016

Uh oh!

ofrobots commented May 4, 2016

Uh oh!

ofrobots commented May 10, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ofrobots commented Apr 26, 2016•
edited
Loading

ChALkeR commented Apr 26, 2016•
edited
Loading