New issue
Advanced search Search tips

Issue 633186 link

Starred by 2 users
Status: Fixed
Owner:
lizeb@chromium.org
Closed: Aug 2016
Cc:
lizeb@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

2.2% regression in system_health.memory_mobile at 408390:408403

Project Member Reported by petrcermak@chromium.org, Aug 1 2016 
See the link to graphs below.

Comment 1 by petrcermak@chromium.org, Aug 1 2016 

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

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


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

android-nexus5
Project Member

Comment 5 by 42576172...@developer.gserviceaccount.com, Aug 2 2016 

Bisect failed: Unknown
Failure reason: the build has failed due to infrastructure failure.
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Aug 6 2016

Cc: lizeb@chromium.org
Owner: lizeb@chromium.org

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

Hi lizeb@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : customtabs: Send time to first contentful paint to the caller.
Author  : lizeb
Commit description:
  
Time to First Contentful Paint is a very useful metric to assess page load
performance. It is collected on the native side. This patch forwards it to the
Java side on Android, and makes it available to the embedding app for Custom
Tabs.

BUG=624909

Review-Url: https://codereview.chromium.org/2139243003
Cr-Commit-Position: refs/heads/master@{#408391}
Commit  : d6fe19b87ba15a78130ec62d87cb0c2c3bae3eaf
Date    : Thu Jul 28 15:01:30 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@408389  1104896  2896.31  8  good
chromium@408390  1103462  3663.57  5  good
chromium@408391  1113088  4239.76  8  bad    <--
chromium@408392  1116570  3663.57  5  bad
chromium@408396  1119846  4670.16  5  bad
chromium@408403  1114931  1831.79  5  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 633186

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_mobile
Test Metric: browse_news-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/browse_news_reddit
Relative Change: 0.89%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3933
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9005164645647577280


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5798413564641280

| 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 8 by lizeb@chromium.org, Aug 8 2016 

I don't think that the CL above is responsible for the increase.

This CL adds a static class in Java, holding a single reference. This reference is going to be always null unless Custom Tabs are used, which I believe is not the case here.

When a Custom Tab is displayed, then this adds an observer. This observer holds a reference to a WebContents (but not to the enclosing activity). I don't think that this reference outlives the WebContents though.

I will upload a CL to make sure this is not happening.
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 9 2016 

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

commit 463006e76317b10db50e6106f60b5dcaa3c3a19f
Author: lizeb <lizeb@chromium.org>
Date: Tue Aug 09 08:20:05 2016

customtabs: Keep a reference to Tab in PageLoadMetricsObserver, not WebContents.

The lifetime of the PageLoadMetricsObserver in CustomTabActivity is tied
to the tab, not to the WebContents. It makes more sense to capture a
reference to the tab.

This might be related to  crbug.com/633186 , even though no evidence of
leak was found. Regardless, this makes more sense.

BUG= 633186 

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

[modify] https://crrev.com/463006e76317b10db50e6106f60b5dcaa3c3a19f/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
Project Member

Comment 11 by 42576172...@developer.gserviceaccount.com, Aug 9 2016 

Bisect failed: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3898
Failure reason: the build has failed due to infrastructure failure.
Project Member

Comment 13 by 42576172...@developer.gserviceaccount.com, Aug 11 2016 


===== BISECT JOB RESULTS =====
Status: failed


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@408389  1107456  6168.33  8  good
chromium@408391  1112576  3752.45  8  good
chromium@408392  1118208  3792.16  8  bad
chromium@408396  1119027  3426.96  5  bad
chromium@408403  1116570  2243.47  5  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 633186

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_mobile
Test Metric: browse_news-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/browse_news_reddit
Relative Change: 0.89%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3967
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004714071504173744


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5281685948596224

| 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!
Project Member

Comment 16 by 42576172...@developer.gserviceaccount.com, Aug 11 2016 


===== BISECT JOB RESULTS =====
Status: failed


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@408389  1107456  6168.33  8  good
chromium@408391  1112576  3752.45  8  good
chromium@408392  1118208  3792.16  8  bad
chromium@408396  1119027  3426.96  5  bad
chromium@408403  1116570  2243.47  5  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 633186

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_mobile
Test Metric: browse_news-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/browse_news_reddit
Relative Change: 0.89%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3967
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004714071504173744


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5281685948596224

| 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!
Project Member

Comment 17 by 42576172...@developer.gserviceaccount.com, Aug 12 2016 


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : customtabs: Send time to first contentful paint to the caller.
Author  : lizeb
Commit description:
  
Time to First Contentful Paint is a very useful metric to assess page load
performance. It is collected on the native side. This patch forwards it to the
Java side on Android, and makes it available to the embedding app for Custom
Tabs.

BUG=624909

Review-Url: https://codereview.chromium.org/2139243003
Cr-Commit-Position: refs/heads/master@{#408391}
Commit  : d6fe19b87ba15a78130ec62d87cb0c2c3bae3eaf
Date    : Thu Jul 28 15:01:30 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@408389  1106739  3426.96  5  good
chromium@408390  1105101  4486.94  5  good
chromium@408391  1113293  1831.79  5  bad    <--
chromium@408392  1117389  5340.53  5  bad
chromium@408396  1114112  0.0      5  bad
chromium@408403  1114931  1831.79  5  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 633186

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_mobile
Test Metric: browse_news-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/browse_news_reddit
Relative Change: 0.74%
Score: 99.0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3978
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004631534325700016


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5801179825569792

| 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 18 by petrcermak@chromium.org, Aug 12 2016

Status: Fixed (was: Assigned)
lizeb: Thanks for looking and fixing this. I can't tell what impact the fix had due to noise (see https://chromeperf.appspot.com/group_report?bug_id=633186). Anyway, we're talking only about a +8 KiB regression, so I think it's safe to mark this as fixed :-)
Project Member

Comment 20 by 42576172...@developer.gserviceaccount.com, Aug 12 2016 


===== BISECT JOB RESULTS =====
Status: completed


=== Bisection aborted ===
The bisect was aborted because The metric values for the initial "good" and "bad" revisions do not represent a clear regression.
Please contact the the team (see below) if you believe this is in error.

=== Warnings ===
The following warnings were raised by the bisect job:

 * Bisect failed to reproduce the regression with enough confidence.

===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@408389  1105920  2896.31  5  good
chromium@408403  1111381  8848.37  6  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 633186

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_mobile
Test Metric: browse_news-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/browse_news_reddit
Relative Change: 0.46%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3979
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004714071504173744


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5281685948596224

| 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!
Project Member

Comment 22 by 42576172...@developer.gserviceaccount.com, Aug 23 2016 


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : customtabs: Send time to first contentful paint to the caller.
Author  : lizeb
Commit description:
  
Time to First Contentful Paint is a very useful metric to assess page load
performance. It is collected on the native side. This patch forwards it to the
Java side on Android, and makes it available to the embedding app for Custom
Tabs.

BUG=624909

Review-Url: https://codereview.chromium.org/2139243003
Cr-Commit-Position: refs/heads/master@{#408391}
Commit  : d6fe19b87ba15a78130ec62d87cb0c2c3bae3eaf
Date    : Thu Jul 28 15:01:30 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N   Good?
chromium@408389  1104213  2738.41  12  good
chromium@408390  1105579  5371.38  12  good
chromium@408391  1112064  2189.4   8   bad    <--
chromium@408392  1117389  3426.96  5   bad
chromium@408396  1116570  3663.57  5   bad
chromium@408403  1119027  4486.94  5   bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 633186

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_mobile
Test Metric: browse_news-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/browse_news_reddit
Relative Change: 1.34%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3956
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004793967593507904


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5838343472939008

| 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 23 by petrcermak@chromium.org, Aug 30 2016

Labels: SystemHealth-Sheriff

Comment 24 by petrcermak@chromium.org, Aug 30 2016

Labels: -Performance-Sheriff
►

Sign in to add a comment