New issue
Advanced search Search tips

Issue 680690 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

6.9% regression in memory.top_10_mobile_stress at 442608:442647

Project Member Reported by benjhayden@chromium.org, Jan 12 2017

Issue description

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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgwP61vAsM


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

android-nexus5X
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jan 12 2017

Cc: boliu@chromium.org
Owner: boliu@chromium.org

=== Auto-CCing suspected CL author boliu@chromium.org ===

Hi boliu@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : boliu
  Commit : fa8bcd12c55c0997520ff969de9e70ab6d0a59dc
  Date   : Tue Jan 10 18:25:08 2017
  Subject: Switch RenderWidgetHostViewAndroid to use Screen

Bisect Details
  Configuration: android_nexus5X_perf_bisect
  Benchmark    : memory.top_10_mobile_stress
  Metric       : memory:chrome:all_processes:reported_by_chrome:gpu:effective_size_avg/background/after_http_m_youtube_com_results_q_science
  Change       : 6.90% | 3941664.0 -> 4213824.0

Revision             Result              N
chromium@442608      3941664 +- 0.0      6      good
chromium@442628      3941664 +- 0.0      6      good
chromium@442633      3941664 +- 0.0      6      good
chromium@442634      4213824 +- 0.0      6      bad       <--
chromium@442635      4213824 +- 0.0      6      bad
chromium@442636      4213824 +- 0.0      6      bad
chromium@442638      4213824 +- 0.0      6      bad
chromium@442647      4213824 +- 0.0      6      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http.m.youtube.com.results.q.science memory.top_10_mobile_stress

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8990637175355090000

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=6174161115807744


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!

Comment 4 by boliu@chromium.org, Jan 12 2017

5x width is 1080, (4213824 - 3941664) / 4 / 1080 is exactly 63, and so that's like that's the height of the top and bottom bars

however, including the top and bottom bar seem like the right thing to do here. otherwise, going into fullscreen and then trying to capture a screenshot will cause a context loss I believe. I'll double check this claim though

Comment 5 by boliu@chromium.org, Jan 18 2017

Ok, probably spent more time on this than I should, but I think I got it..

The shared memory is coming out of the mapped memory pool. The change had two affects on that pool

1) the reclaim limit is made a higher, ie the amount of unused memory before things are freed
2) the chunk size is made a higher. pool always allocates in multiples of the chunk size

The interesting thing here is the readback is scaled by 1/2 (on n5x anyway, so 1/4 of memory), and the chunk size is also divided by 4. So really, the additional overhead here came from chunk size getting bigger when making a single allocation that fits into a single chunk

So afaict, there is no reason why the reclaim limit, or the chunk size has to fit a single screen (ignoring the 1/2 scale for the moment). I'll just make it lower
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 19 2017

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

commit b9ee881336aafb25b77a02b0620605696e2200bc
Author: boliu <boliu@chromium.org>
Date: Thu Jan 19 01:03:53 2017

android: Tweak readback context memory limits

This context is used for readbacks only, and is destroyed after all
readbacks are done. And readback memory comes from the mapped memory
pool.

Lower the transfer buffer pool to 128k. The readback memory doesn't come
from this pool, so there is no reason it's computed from the screen size.

Lower the mapped memory reclaim limit to 4k. Note this is not the
maximum allocation size, so there is no correctness issue here. It also
lowers the chunk size, ie allocations are rounded up to a multiple of
the chunk. There are very few (only one most of the time) allocations
from this pool, having a lower chunk size should not have have a big
impact on perf (from reuse), but lowers memory overhead.

BUG= 680690 

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

[modify] https://crrev.com/b9ee881336aafb25b77a02b0620605696e2200bc/content/browser/renderer_host/render_widget_host_view_android.cc

Comment 7 by boliu@chromium.org, Jan 19 2017

Status: Fixed (was: Untriaged)
Labels: Performance-Memory

Sign in to add a comment