New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 771966 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug



Sign in to add a comment

Unmapping of memory chunks can be blocked by sweeping

Project Member Reported by u...@chromium.org, Oct 5 2017

Issue description

Sweeper is not making progress may cause accumulation of pages in the unmapper queue.
 
Labels: NodeJS-Backport-Review

Comment 2 by u...@chromium.org, Oct 5 2017

Labels: M-63 M-60 M-61
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/2c75616028e75883e28df2c6d8dd2a6bfe47f718

commit 2c75616028e75883e28df2c6d8dd2a6bfe47f718
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Thu Oct 05 14:38:31 2017

[heap] Ensure progress in unmapping memory chunks.

If sweeping is not making progress and there are many young generation
GCs happening, then this can lead to accumulation of memory chunks in
the unmapper queue.

Bug:  chromium:771966 
Change-Id: Ief73ada0d17198a80b668850c6d2e7ea413113e7
Reviewed-on: https://chromium-review.googlesource.com/702479
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48312}
[modify] https://crrev.com/2c75616028e75883e28df2c6d8dd2a6bfe47f718/src/heap/heap.cc
[modify] https://crrev.com/2c75616028e75883e28df2c6d8dd2a6bfe47f718/src/heap/spaces.h

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/676c41321a948c08d6a43de43fe9b1d60ae81c00

commit 676c41321a948c08d6a43de43fe9b1d60ae81c00
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Thu Oct 05 16:21:12 2017

[heap] Fix threshold for delayed chunks after 2c7561.

Bug:  chromium:771966 
Change-Id: Iac5ee55c0d31de477f21f091f4be015a1ca8d00c
Reviewed-on: https://chromium-review.googlesource.com/702382
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48316}
[modify] https://crrev.com/676c41321a948c08d6a43de43fe9b1d60ae81c00/src/heap/heap.cc

Comment 5 by u...@chromium.org, Oct 5 2017

Labels: -M-63 M-62
I'll request merge to 60-62 once this gets Canary coverage.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e0d64dc67c6abf36a37efdc6e8e6903bb114ebd3

commit e0d64dc67c6abf36a37efdc6e8e6903bb114ebd3
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Fri Oct 06 11:16:21 2017

[heap] Print the number of chunks in unmapper queue in --trace-gc-nvp

Bug:  chromium:771966 
Change-Id: I146b279c4713b7dd716c6d55ca5e6c6e23a3ad7e
Reviewed-on: https://chromium-review.googlesource.com/704740
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48338}
[modify] https://crrev.com/e0d64dc67c6abf36a37efdc6e8e6903bb114ebd3/src/heap/gc-tracer.cc
[modify] https://crrev.com/e0d64dc67c6abf36a37efdc6e8e6903bb114ebd3/src/heap/spaces.cc
[modify] https://crrev.com/e0d64dc67c6abf36a37efdc6e8e6903bb114ebd3/src/heap/spaces.h

Cc: ofrobots@google.com

Comment 8 by ofrobots@google.com, Oct 19 2017

ulan@: does this have enough canary coverage to be merged back?

Comment 9 by u...@chromium.org, Oct 19 2017

Cc: hablich@chromium.org
Labels: Merge-Request-62 Merge-Request-61 Merge-Request-60
There is enough coverage. Requesting merges to M60, M61, 62.

Michael, could you please review the merge request?
Project Member

Comment 10 by sheriffbot@chromium.org, Oct 19 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Please add appropriate OSs.

Comment 12 by u...@chromium.org, Oct 19 2017

candrada@, the issue affects all OSs.
ulan@: do you expect this issue affect V8 5.1? That is, I am trying to figure out if we need to manually back port for Node 6.x on the Node side.

From Node 8.x+ point of view, we only need the back port to 6.1 and 6.2 (V8 6.0 is no longer relevant to Node.js).

Comment 14 by u...@chromium.org, Oct 19 2017

Labels: -M-60 -Merge-Request-60
5.1 is not affected.
ulan@ can you please specifically mark all impacted OS's? This will help with our reporting and ensuring the right owner can view it. I know you've mentioned it impacts all OS's, but please mark it in the bug as well. 

Can you please comment why this is urgent and critical for M62? M62 is already in stable rollout for Desktop, Android, and IOS, so the requirements are a bit stricter for merges. go/chrome-merges for more details. 

Comment 16 by habl...@google.com, Oct 20 2017

Labels: -Merge-Request-61 Merge-Rejected-61
61 is already deprecated.
Labels: -Merge-Review-62 Merge-Rejected-62
Node can float this one and it is a Pri=3. Let's not merge it.
Cc: yangguo@chromium.org
I am suspecting that this is also causing
  https://github.com/nodejs/help/issues/917
Labels: -NodeJS-Backport-Review NodeJS-Backport-Done
Back port for Node: https://github.com/nodejs/node/pull/16490

Comment 20 by u...@chromium.org, Nov 29 2017

Owner: mlippautz@chromium.org
mlippautz@ is working on decoupling the new generation sweeping from the old generation sweeping.
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Status: Started (was: Assigned)
Status: Fixed (was: Started)
Just realized that this bug was also used for tracking backmerges. I will open a new one for the improved feature.

Sign in to add a comment