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

Issue 690229 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 725265



Sign in to add a comment

28.6%-100% regression in memory.top_10_mobile at 448699:448828

Project Member Reported by briander...@chromium.org, Feb 9 2017

Issue description

See the link to graphs below.
 
Cc: alex...@chromium.org
Owner: alex...@chromium.org

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

Hi alexmos@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 : alexmos
  Commit : 443064bb9932a98ad47c1cbbf8bba2493a31f888
  Date   : Tue Feb 07 22:51:33 2017
  Subject: Turn on AreCrossProcessFramesPossible by default.

Bisect Details
  Configuration: android_webview_arm64_aosp_perf_bisect
  Benchmark    : memory.top_10_mobile
  Metric       : memory:webview:all_processes:reported_by_chrome:gpu:effective_size_avg/foreground/http_www_baidu_com_s_word_google
  Change       : 100.00% | 65536.0 -> 131072.0

Revision             Result              N
chromium@448748      65536.0 +- 0.0      6      good
chromium@448757      65536.0 +- 0.0      6      good
chromium@448760      65536.0 +- 0.0      6      good
chromium@448761      131072 +- 0.0       6      bad       <--
chromium@448762      131072 +- 0.0       6      bad
chromium@448766      131072 +- 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-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile

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

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


| 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!
Cc: creis@chromium.org nick@chromium.org nasko@chromium.org
Components: Internals>Sandbox>SiteIsolation
Labels: OS-Android
Status: Assigned (was: Untriaged)
Yikes.  I guess we never ran with AreCrossProcesFramesPossible turned on by default on Android (since --isolate-extensions was always off there).  We'll have to investigate this further (especially for TDI), and for now perhaps we should turn off AreCrossProcesFramesPossible on Android.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 10 2017

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

commit a4c553be55b1261fd0beda3ea8b527b787ab50a1
Author: alexmos <alexmos@chromium.org>
Date: Fri Feb 10 00:39:26 2017

Turn off SiteIsolationPolicy::AreCrossProcessFramesPossible for Android.

AreCrossProcessFramesPossible was turned on by default for all
platforms in https://codereview.chromium.org/2676823003, but that
shouldn't have included Android, which still doesn't have input event
routing fully implemented.  Additionally, turning it on revealed a
performance regression on Android that will need to be investigated.

BUG= 690229 , 545200

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

[modify] https://crrev.com/a4c553be55b1261fd0beda3ea8b527b787ab50a1/content/common/site_isolation_policy.cc

Labels: -Performance-Sheriff -M-58 Proj-TopDocumentIsolation-BlockingLaunch
Owner: ----
Status: Available (was: Assigned)
I've verified that after #5 turned off AreCrossProcesFramesPossible for Android, all the graphs in https://chromeperf.appspot.com/group_report?bug_id=690229 went back to normal.  So the regression is fixed on trunk, but I'll keep this open so that someone from the Site Isolation side can investigate this further.  Not sure it'll be me, so I'll mark this as available for now.  Also adding the TDI label as this will need to be addressed before TDI can ship on Android.
A related issue was filed for Android webview: https://crbug.com/691592.  There, it seems that turning on AreCrossProcesFramesPossible actually caused some improvements.
Blocking: 725265
Project Member

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

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

commit 00d432064a6fcd6252211a938840efb8e36b463f
Author: kenrb <kenrb@chromium.org>
Date: Mon Jun 26 18:37:11 2017

Make AreCrossProcessFramesPossible return true on Android

The function AreCrossProcessFramePossible returns true on all platforms
but Android, because of some memory regressions that were previously
observed. The regressions do not currently manifest on perf trybots, so
this is another attempt to make the change on Android, in the hope that
the underlying problem has since been resolved.

This also includes some changes to gesture event routing on touchscreen
devices that are necessary to fix test breakage when input event
routing for Android is always turned on. We now route gesture events
even if there have been no corresponding touch events preceding them.

If no regressions are detected, the next step will be to remove the function from the code base.

BUG= 690229 

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

[modify] https://crrev.com/00d432064a6fcd6252211a938840efb8e36b463f/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/00d432064a6fcd6252211a938840efb8e36b463f/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/00d432064a6fcd6252211a938840efb8e36b463f/content/common/site_isolation_policy.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 10 2017

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

commit c3de9c1ad085f8a2210eb290b719468be08323c3
Author: kenrb <kenrb@chromium.org>
Date: Mon Jul 10 20:17:38 2017

Enable AreCrossProcessFramesPossible on Android

Previous attempts to enable cross process frames on Android have caused
changes in memory usage on Android Webview tests, resulting in them
being reverted. This might be because the flag changed the behavior of
--single-process, which is no longer the case since r485292 landed.

The regressed benchmark was memory.top_10_mobile under Android Webview
tests. We expect that the problem is now fixed and we can land this
without causing regressions, but this is still a speculative attempt.

BUG= 690229 , 737264

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

[modify] https://crrev.com/c3de9c1ad085f8a2210eb290b719468be08323c3/content/common/site_isolation_policy.cc

Comment 11 by kenrb@chromium.org, Jul 10 2017

Owner: kenrb@chromium.org
Status: Assigned (was: Available)

Comment 12 by kenrb@chromium.org, Jul 27 2017

Status: Fixed (was: Assigned)

Sign in to add a comment