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

Issue 722858 link

Starred by 3 users

Issue metadata

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


Show other hotlists

Hotlists containing this issue:
system-health-regressions


Sign in to add a comment

1.3%-9.6% regression in memory.top_10_mobile at 471896:471943

Project Member Reported by lanwei@chromium.org, May 16 2017

Issue description

See the link to graphs below.
 

Comment 1 by lanwei@chromium.org, May 16 2017

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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmoflvgsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmtHUqAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmoin_AkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmoin_AoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmvapswkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6p3PwgsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6vvCgAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmrGoogkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6p3PwgkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmuzv5QoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6t_SuwkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmo6vqQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6vvCgAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmuzv5QgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmuaUpgsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmrHcswoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmuzvpQsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6oD39wsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6oD39wkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6pfSoQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6uOL5AkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmvygogoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmtCR8QsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6p-_swoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmpW2rAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6rSy0gsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmpitoQsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6vuC_wkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmqKr8QkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6ojC1ggM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmsjlpwkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmpbrvAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgqq7nqAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmuWL6AoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmvKJoAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6tOQ5gsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6oeF4wkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgqq7nqAgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6p3PwgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6q-xsgsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmqjJ6AgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmrbw_gkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmoOTvAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmpi7sAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6sfo-gkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmqHJ8AoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmrbVuQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmry7vQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmoqguQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6qWi8AkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmuLnkgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg6rHwsQsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmuWwtgkM


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

android-webview-nexus5X
android-webview-nexus6
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, May 18 2017

Cc: fgor...@chromium.org
Owner: fgor...@chromium.org

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

Hi fgorski@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 : fgorski
  Commit : 7c52dcabdaefff4b583999c786afbefa907564fb
  Date   : Mon May 15 22:15:20 2017
  Subject: [Android] Adding scheduling through GcmNetworkManager

Bisect Details
  Configuration: android_webview_arm64_aosp_perf_bisect
  Benchmark    : memory.top_10_mobile
  Metric       : memory:webview:all_processes:reported_by_os:system_memory:java_heap:proportional_resident_size_avg/background/after_http_en_m_wikipedia_org_wiki_Science
  Change       : 9.97% | 1438208.0 -> 1581568.0

Revision             Result                  N
chromium@471895      1438208 +- 13410.1      6      good
chromium@471912      1444352 +- 4031.49      6      good
chromium@471916      1477291 +- 3739.12      6      good
chromium@471918      1444352 +- 4031.49      6      good
chromium@471919      1585664 +- 5369.9       6      bad       <--
chromium@471920      1592661 +- 40771.8      6      bad
chromium@471928      1581568 +- 5369.9       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/8979426804332057216

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


| 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 Speed>Bisection.  Thank you!
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, May 18 2017

 Issue 723017  has been merged into this issue.
Project Member

Comment 5 by 42576172...@developer.gserviceaccount.com, May 18 2017

 Issue 723018  has been merged into this issue.
Project Member

Comment 6 by 42576172...@developer.gserviceaccount.com, May 19 2017

Issue 724033 has been merged into this issue.
This looks weird. The code that was added is not yet invoked. And even when it will get invoked it would happen very rarely.

What OS the webview bots in question running?
Status: Assigned (was: Untriaged)
Specific OS? The main perf bots are running M.

Bisectors also M (you can see on the device_status step)
Great.

Most of the code does not even run then, because I am adding parts that are every used in pre-M (else clause below):

    private static BackgroundTaskSchedulerDelegate getSchedulerDelegate() {
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
            return new BackgroundTaskSchedulerJobService();
        } else {
            return new BackgroundTaskSchedulerGcmNetworkManager();
        }
    }

The only change that is affecting Chrome memory in run-time would perhaps be adding an extra GCMTaskService, but that was not added to manifest until later:
https://chromium.googlesource.com/chromium/src/+/df49c6da32ca2a0ed87e1d5f87f4032d9c4fdc12

I also don't understand how this would affect webview.
Comment repeated from issue 724087

This patch is first in a cycle of refactorings that attempt to centralize background scheduling. Because most of the newly added code is not even run (with exception of storing information in shared preferences), there is no more impact on Heap then class metadata (4 classes) and a few static variables:

BackgroundTaskGcmTaskService
private static final String TAG = "BkgrdTaskGcmTS";

BackgroundTaskGcmTaskService::Waiter
private static final long MAX_TIMEOUT_SECONDS = 179;

BackgroundTaskSchedulerGcmNetworkManager
private static final String TAG = "BkgrdTaskSchedGcmNM";
static final String BACKGROUND_TASK_CLASS_KEY = "_background_task_class";
static final String BACKGROUND_TASK_EXTRAS_KEY = "_background_task_extras";

BackgroundTaskSchedulerPrefs
private static final String TAG = "BTSPrefs";
private static final String KEY_SCHEDULED_TASKS = "bts_scheduled_tasks";
private static final String KEY_LAST_OS_VERSION = "bts_last_os_version";
private static final String KEY_LAST_APP_VERSION = "bts_last_app_version";
private static final String ENTRY_SEPARATOR = ":"; 

I suspect (don't know enough about Perm Gen allocation) that if Heap is growing in blocks, this CL caused a new block of memory to be added to what Chrome was already using ( issue 722858 ). Same thing probably happened to apk sizes (issue 724087). The following CL is the first one that takes advantage of refactoring, and therefore will hopefully start paying for it: https://codereview.chromium.org/2830843002/

It's delta is at least 5 classes cut, and a lot of duplicated functionality removed.

I recommend closing this bug as a won't fix. Thoughts?
Status: Started (was: Assigned)
So, could we justify this as an intermediate state of some refactoring that's going to reap some benefits down the line? If so, that's ok with me, but what is the ETA on the full set of patches?
The code adding possibility to schedule tasks using GCMNetworkManager (on pre-M versions of Android) has already been landed in 4 patches:
https://codereview.chromium.org/2779753002/  <-- original offending patch
https://codereview.chromium.org/2814653003/
https://codereview.chromium.org/2825783003/
https://codereview.chromium.org/2819703002/

The next patch mentioned in #10 is going to be the first one taking advantage of the common code:
https://codereview.chromium.org/2830843002/
Cc: nyquist@chromium.org
Adding Tommy to this bug as well, as he is on board with the current plan as well, per https://bugs.chromium.org/p/chromium/issues/detail?id=724087#c8
Project Member

Comment 15 by 42576172...@developer.gserviceaccount.com, May 27 2017

Issue 726888 has been merged into this issue.
Revision 476352 is expected to have positive impact on the problem. I'll be following up on this.
fgorski: is this fixed?
We have yet to move various components to background_task_scheduler.

There is not going to be an explicit fix for this issue however, as there is really nothing to fix.
Components: Internals>BackgroundTaskScheduler
Status: WontFix (was: Started)
I actually see more memory usage in r476352, but overall the regressions from this issue are less than 1MiB and per #18 it doesn't have an explicit fix, so marking WontFix.

Sign in to add a comment