New issue
Advanced search Search tips

Issue 908359 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 28
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1.8%-9.7% regression in system_health.memory_mobile at 610606:610653

Project Member Reported by rmcilroy@chromium.org, Nov 26

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=908359

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=dbc478e49aa6b8a55551579dc9f94bb663fc37701df10aa701fad171ecdf66dd


Bot(s) for this bug's original alert(s):

Android Nexus5 Perf

system_health.memory_mobile - Benchmark documentation link:
  https://bit.ly/system-health-benchmarks

memory.top_10_mobile - Benchmark documentation link:
  None
Cc: verwa...@chromium.org
Owner: verwa...@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/123c458fe40000

[zone] Use 32kb instead of 1MB as high zone page size by verwaest@chromium.org
https://chromium.googlesource.com/v8/v8/+/a04862021eee7708335eea0ce81531f999b5e66c
memory:chrome:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size: 6.282e+07 → 6.905e+07 (+6.228e+06)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  https://bit.ly/system-health-benchmarks
Looks like a decent regression Toon, wasn't this CL meant to reduce memory usage?
That looks pretty bad indeed. It was more about performance than memory; although by giving control to malloc and allocating smaller pages the hope was that it might also improve memory. Seems like it doesn't. Will revert and we should probably think about how to get back some of the performance benefits without the memory overhead. Since it seems malloc too aggressively pools and/or preallocates, that likely affects the rest of Chrome as well...
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 27

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

commit de20e6d3a855d7dc4a991dd192052c30b6ea996e
Author: Toon Verwaest <verwaest@chromium.org>
Date: Tue Nov 27 12:55:45 2018

[zone] Revert to previous zone allocation strategies due to severe memory regressions.

Unfortunately the previous strategy was slower but more memory efficient. For now simply revert.

Revert "[zone] Use 32kb instead of 1MB as high zone page size"
Revert "[zone] Get rid of the Zone's segment pool"
Revert "[zone] Further simplify zone expansion, use single default page size"

Bug:  chromium:908359 
Change-Id: I649542e7e61eef0c14a26ffd21039e8340ab4d04
Reviewed-on: https://chromium-review.googlesource.com/c/1351027
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57872}
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/src/api.cc
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/src/builtins/setup-builtins-internal.cc
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/src/heap/heap.cc
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/src/isolate.cc
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/src/zone/accounting-allocator.cc
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/src/zone/accounting-allocator.h
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/src/zone/zone-segment.h
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/src/zone/zone.cc
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/src/zone/zone.h
[modify] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/test/unittests/BUILD.gn
[add] https://crrev.com/de20e6d3a855d7dc4a991dd192052c30b6ea996e/test/unittests/zone/segmentpool-unittest.cc

Cc: m...@chromium.org
 Issue 909057  has been merged into this issue.
Status: Fixed (was: Assigned)

Sign in to add a comment