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

Issue 724947 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

external fragmentation in map space and code space increased significantly

Project Member Reported by jochen@chromium.org, May 22 2017

Issue description

Thanks Jochen!

I believe this is due bf74d43de055393d97a3db4f01e4c832e35af10f which removes accounting for uncommitted memory when shrinking pages, because it actually is incorrect. This leaves us in a state where we properly shrink a page but do not account for the memory we are not able to use anymore.

Going forward, we should either
(a) Fix unaccounting of uncommited memory for shrinked pages. This will require one more field on page to keep track of the uncommitted area.
(b) Remove shrinking if we feel that it doesn't give us any benefit.

Comment 2 by jochen@chromium.org, May 23 2017

Owner: hpayer@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by hpayer@chromium.org, May 30 2017

Michael, adding another counter makes sense. Let's make sure that the accounting is correct. The UMA counters are important and hopefully recover after fixing this. Can you take care of this?
Cc: -mlippautz@chromium.org
Labels: -Pri-3 Pri-2
Owner: mlippautz@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, May 31 2017

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

commit 0d06e42b690cfe80454c7f065c382f2b7200a40f
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Wed May 31 20:12:55 2017

[heap] Use partial free when shrinking instead of uncommitting

This fixes the counter inconsistencies while leaving the memory in an
inaccessible state.

Bug:  chromium:724947 
Change-Id: I431eb6fda84922a52dfb9380c6b482ada55bccee
Reviewed-on: https://chromium-review.googlesource.com/519164
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45647}
[modify] https://crrev.com/0d06e42b690cfe80454c7f065c382f2b7200a40f/src/heap/spaces.cc
[modify] https://crrev.com/0d06e42b690cfe80454c7f065c382f2b7200a40f/src/heap/spaces.h

This one should be fixed wit #6. I will keep it open and observe the counters.
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 6 2017

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

commit c8e6cdfdcee0319fec69926e074d047f668c6f8b
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Tue Jun 06 14:16:55 2017

Revert "[heap] Use partial free when shrinking instead of uncommitting"

This reverts commit 0d06e42b690cfe80454c7f065c382f2b7200a40f.

Reason for revert: clusterfuzz and canary crashes.

BUG= chromium:729209 , v8:6456 

Original change's description:
> [heap] Use partial free when shrinking instead of uncommitting
> 
> This fixes the counter inconsistencies while leaving the memory in an
> inaccessible state.
> 
> Bug:  chromium:724947 
> Change-Id: I431eb6fda84922a52dfb9380c6b482ada55bccee
> Reviewed-on: https://chromium-review.googlesource.com/519164
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Hannes Payer <hpayer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#45647}

TBR=hpayer@chromium.org,mlippautz@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug:  chromium:724947 

Change-Id: I6c52b478b89a858ba984fe17f86cdf15fcfa974c
Reviewed-on: https://chromium-review.googlesource.com/525716
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45733}
[modify] https://crrev.com/c8e6cdfdcee0319fec69926e074d047f668c6f8b/src/heap/spaces.cc
[modify] https://crrev.com/c8e6cdfdcee0319fec69926e074d047f668c6f8b/src/heap/spaces.h

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 13 2017

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

commit 1b4934b9ce7aae28a39f56ff577311278c7615db
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Tue Jun 13 09:30:30 2017

Reland "[heap] Use partial free when shrinking instead of uncommitting"

This fixes the counter inconsistencies and makes use of the already existing
mechanism for partially releasing memory.

This reverts commit c8e6cdfdcee0319fec69926e074d047f668c6f8b.

Bug:  chromium:724947 
Change-Id: I2a7b52a28654fd2524df502a353997393d4f53ac
Reviewed-on: https://chromium-review.googlesource.com/530369
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45890}
[modify] https://crrev.com/1b4934b9ce7aae28a39f56ff577311278c7615db/src/heap/spaces.cc
[modify] https://crrev.com/1b4934b9ce7aae28a39f56ff577311278c7615db/src/heap/spaces.h

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 13 2017

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

commit 041d7339e3ade7bdcac7316c06fdaed66a142da4
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Tue Jun 13 18:55:53 2017

[heap] Fix memory allocator counters for partially releasing pages

Bug:  chromium:724947 
Change-Id: I287677b2cf18154bcbc0d0a5b15d12455d73d0c3
Reviewed-on: https://chromium-review.googlesource.com/534153
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45923}
[modify] https://crrev.com/041d7339e3ade7bdcac7316c06fdaed66a142da4/src/base/platform/platform.h
[modify] https://crrev.com/041d7339e3ade7bdcac7316c06fdaed66a142da4/src/heap/spaces.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 14 2017

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

commit 21389501f56789cb414329d5c6cb9a7ffd94f8e3
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Wed Jun 14 15:18:01 2017

[heap] Fix adjusting of area end when shrinking large pages

Bug:  chromium:733059 ,  chromium:724947 
Change-Id: Id7abc22ee0975cd609cc06a02552f68e9e0077e8
Reviewed-on: https://chromium-review.googlesource.com/535596
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45952}
[modify] https://crrev.com/21389501f56789cb414329d5c6cb9a7ffd94f8e3/src/heap/spaces.cc
[modify] https://crrev.com/21389501f56789cb414329d5c6cb9a7ffd94f8e3/src/heap/spaces.h
[add] https://crrev.com/21389501f56789cb414329d5c6cb9a7ffd94f8e3/test/mjsunit/regress/regress-733059.js

Labels: OS-All
Status: Fixed (was: Started)
This is fixed now. 

The overall improvements can be found at 
  https://chromeperf.appspot.com/group_report?rev=478996
Cc: u...@chromium.org mlippautz@chromium.org
 Issue 719025  has been merged into this issue.

Sign in to add a comment