New issue
Advanced search Search tips

Issue 800743 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

14.8% regression in system_health.memory_mobile at 527663:527815

Project Member Reported by hjd@google.com, Jan 10 2018

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Jan 10 2018

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=800743

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


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

android-one
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jan 10 2018

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

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

Hi ericrk@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 : Eric Karl
  Commit : c0a0cf3967bf4481fbc18e5012219cf5da2edddb
  Date   : Mon Jan 08 20:54:39 2018
  Subject: Temporarily disable context-clear on background for low-end Android

Bisect Details
  Configuration: android_one_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:chrome:all_processes:reported_by_chrome:malloc:effective_size_avg/background_social/background_social_facebook
  Change       : 14.89% | 10948372.0 -> 12578394.6667

Revision             Result                   N
chromium@527662      10948372 +- 104971       6      good
chromium@527739      10926632 +- 35941.0      6      good
chromium@527744      10960369 +- 200705       6      good
chromium@527747      10910149 +- 184924       6      good
chromium@527748      12581015 +- 12242.4      6      bad       <--
chromium@527749      12585573 +- 39920.6      6      bad
chromium@527758      12568693 +- 86977.1      6      bad
chromium@527777      12595199 +- 93242.1      6      bad
chromium@527815      12578395 +- 165348       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=background.social.facebook system_health.memory_mobile

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8957781946848972160


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 10 2018

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

commit 465a525298a32c85cadc184a793f5d7bf7c54fab
Author: Eric Karl <ericrk@chromium.org>
Date: Wed Jan 10 21:16:27 2018

Reland "Improve timing of placeholder image on Chrome resume""

Improve timing of placeholder image on Chrome resume

Currently, when Chrome is foregrounded, Android OS shows a placeholder
image of the last screen rendered. It does this until Chrome fires the
surfaceRedrawAsync callback. Chrome currently does this as soon as it
produces its first frame after foregrounding.

Unfortunately, Chrome doesn't wait for renderer content before
producing a frame, so the first frame may include a blank renderer.
This is especially common on low-end devices which produce frames
more slowly. In these cases, we see the placeholder, then a blank
renderer, then a populated renderer, which produces a flickering
effect.

This CL ports the ui/compositor's compositor lock to the android
CompositorImpl.

If Chrome is producing the first frame since becoming visible, we
take this lock when a renderer first connects to the
browser, and release it when the renderer produces a frame. This
prevents the browser from producing a frame until it has renderer
content (or a failsafe timeout has been reached).

Note that the VrCompositor used to provide similar functionality by
calling DeferComimits direclty. This code wasn't working as intended
and after discussion with the VR team has been removed.

TBR=ccameron@chromium.org,mthiesse@chromium.org,bajones@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Bug:  792120 ,  800743 
Change-Id: I69b2c0d7da9c5e132f02a6ae951be8846889607c
Reviewed-on: https://chromium-review.googlesource.com/857836
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528425}
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/chrome/browser/android/vr_shell/vr_compositor.cc
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/chrome/browser/android/vr_shell/vr_compositor.h
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/content/browser/BUILD.gn
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/content/browser/android/overscroll_controller_android_unittest.cc
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/content/public/browser/android/compositor.h
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/gpu/ipc/service/gpu_channel_manager.cc
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/ui/android/BUILD.gn
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/ui/android/DEPS
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/465a525298a32c85cadc184a793f5d7bf7c54fab/ui/android/window_android_compositor.h

Comment 5 by hjd@chromium.org, Jan 11 2018

Status: Fixed (was: Assigned)
Great! The graphs all recovered in the range which contained the CL above.

I'll marked this as fixed unless you think there is more work here?

Sign in to add a comment