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

Issue 635491 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 635492
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

9.5% regression in system_health.memory_mobile at 410004:410049

Project Member Reported by petrcermak@chromium.org, Aug 8 2016

Issue description

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

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


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

android-one
Project Member

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

Cc: r...@opera.com
Owner: r...@opera.com

=== Auto-CCing suspected CL author rune@opera.com ===

Hi rune@opera.com, 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 : Revert of Add a fast-path for independent inherited properties (patchset #13 id:240001 of https://codereview.chromium.org/2117143003/ )
Author  : rune
Commit description:
  
Reason for revert:
Caused issues  634254  and  633859 .

Original issue's description:
> Add a fast-path for independent inherited properties
>
> Add a fast-path for inherited properties which do not depend on and do
> not affect any other properties on ComputedStyle. When these properties
> are modified in a parent element, set them directly on ComputedStyle and
> skip doing a full recalc for elements only affected by this change.
>
> Also implemented two of these properties: visibility and pointer-events,
> storing an extra 2 bits per ComputedStyle. This increases the size of
> ComputedStyle by 1 byte on Windows and some Android builds (due to
> aligned fields), which increases the memory usage for a standard page
> with ~1000 elements by up to 1kb (although potentially up to 4/8kb on
> 32/64 bit builds due to packing, although this depends on the allocator
> implementation details) but realistically less since style sharing only
> creates one ComputedStyle object for each unique style.
>
> Benchmarks show a speed increase of up to 2x for setting these
> properties on the root element of a typical web page (Facebook, Twitter,
> Pinterest, Amazon, Wikipedia) and letting the change propagate directly
> onto the child ComputedStyle objects, rather than doing a full style
> recalc.
>
> Initial Benchmarks:
> https://docs.google.com/spreadsheets/d/1mUuJEs8cPWyNTR7tQw27oxq6fDTvWiAwgatf_g--B4w/edit#gid=1597242813
>
> Follow-up Benchmarks:
> https://docs.google.com/spreadsheets/d/1mUuJEs8cPWyNTR7tQw27oxq6fDTvWiAwgatf_g--B4w/edit#gid=918856082
>
> BUG=622138
>
> Committed: https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97
> Cr-Commit-Position: refs/heads/master@{#409143}

TBR=esprehn@chromium.org,meade@chromium.org,timloh@chromium.org,sashab@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=622138

Review-Url: https://codereview.chromium.org/2213223004
Cr-Commit-Position: refs/heads/master@{#410030}
Commit  : bf9f7aeb812949577490b55e40dd36c8eca7b8f9
Date    : Fri Aug 05 11:23:41 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@410003  2698445  6075.35  5  good
chromium@410027  2714010  3663.57  5  good
chromium@410029  2705818  27870.9  5  good
chromium@410030  3063808  2896.31  5  bad    <--
chromium@410033  3067904  2896.31  5  bad
chromium@410038  2968781  50031.6  5  bad
chromium@410049  2896691  91332.5  5  bad

Bisect job ran on: android_one_perf_bisect
Bug ID: 635491

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: load_search-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/load_search_google
Relative Change: 7.35%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1520
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004243273601718944


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

| 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 5 by r...@opera.com, Aug 16 2016

Status: WontFix (was: Assigned)
Looking at the graphs, this report is just bogus.

Cc: primiano@chromium.org perezju@chromium.org picksi@chromium.org amineer@chromium.org
Labels: -Pri-2 OS-Android Pri-1
Owner: petrcermak@chromium.org
rune: Could you please be more specific about what you mean by "bogus"?

I agree that there are two strange things:

  1. The blamed patch is a revert and the original patch didn't cause an
     improvement. This means that the actual regression might have been
     caused by some intermediate patch.

  2. The value for the reference build has improved roughly as much as
     the regular build regressed.

However:

  1. The numbers from the bisect match the ones on the dashboard. This
     indicates that the alert was not due to a hardware/software change
     on the bots.

  2. This is a clear +288.0 KiB regression in a top-level metric
     on a #1 Alexa top global site reported by the OS for a low-end
     device. I would in fact go as far as marking this a release blocker
     (+cc amineer,perezju: WDYT?).

Let's bisect the reference build improvement to get more insight.
Status: Assigned (was: WontFix)
Cc: -picksi@chromium.org -amineer@chromium.org -primiano@chromium.org
Labels: -Pri-1 Pri-2
As far as I can tell, bisects are not supported for reference builds (+cc sullivan). I'll at least run another regular bisect. I've just noticed that r410036 is in the bisected range, so this is most probably a false alarm due to SH story set change. If that's the case, sorry for making so much fuss about it.
Cc: sullivan@chromium.org

Comment 11 by r...@opera.com, Aug 16 2016

Oh, I was looking at the wrong graph. I thought the regression was marked in the green one, but it's they grey one.

Comment 12 by r...@opera.com, Aug 16 2016

One thing to notice is that when the reverted CL was introduced, it didn't cause any memory improvements.

Cc: robert...@chromium.org dtu@chromium.org pras...@chromium.org
Double-checking with dtu, prasadv, robertocn: I think there is no way to bisect the reference build? Dave, do we log the ref build somewhere? I think the level of detail we're capable of getting is "Chrome updated from M49 to M51"
Project Member

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


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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@410003  2693530  6211.89  5  good
chromium@410027  2717286  3663.57  5  good
chromium@410049  3004006  33850.9  5  bad

Bisect job ran on: android_one_perf_bisect
Bug ID: 635491

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: load_search-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/load_search_google
Relative Change: 11.53%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1522
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004166446497197776


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

| 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!
The bisect failed due to a timeout (24 hours). I started another one to continue.
Project Member

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


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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@410003  2693530  6211.89  5  good
chromium@410027  2717286  3663.57  5  good
chromium@410049  3004006  33850.9  5  bad

Bisect job ran on: android_one_perf_bisect
Bug ID: 635491

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: load_search-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/load_search_google
Relative Change: 11.53%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1522
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004166446497197776


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

| 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 19 by dtu@chromium.org, Aug 17 2016

Cc: aiolos@chromium.org
Kari said to look for changes to this file for ref build updates.
https://github.com/catapult-project/catapult/commits/master/catapult_base/catapult_base/chrome_binaries.json
Looks like there was no ref build change on Aug 5.

I don't think there's a way to bisect on the reference build.

I've attached a plot of the bisect results. It looks like a regression at r410030, and a an improvement (and noise increase) in the range of r410034-r410038.
plot.png
93.0 KB View Download
Project Member

Comment 21 by 42576172...@developer.gserviceaccount.com, Aug 19 2016


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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@410003  2701722  8493.65  5  good
chromium@410027  2709094  4670.16  5  good
chromium@410038  2918810  95779.9  5  bad
chromium@410049  2922086  98755.2  5  bad

Bisect job ran on: android_one_perf_bisect
Bug ID: 635491

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: load_search-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/load_search_google
Relative Change: 8.16%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1533
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004166446497197776


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

| 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 dtu@chromium.org, Aug 19 2016

system_health.memory_mobile looks like it takes a long time to run (25-40 minutes). Is it possible to break it up into smaller benchmarks?

That last run, looks like the test was pretty flaky or failed?
Project Member

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


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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@410027  2708275  9340.32  5  good
chromium@410033  2716160  7073.33  8  good
chromium@410036  2740224  675767   8  bad
chromium@410038  3037184  213015   8  bad

Bisect job ran on: android_one_perf_bisect
Bug ID: 635491

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: load_search-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/load_search_google
Relative Change: 13.22%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1541
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9003880583206099456


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

| 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!
Blocking: 589726
#23: Instead of breaking it up into smaller benchmarks, we would like to be able to bisect on individual stories.
Blocking: -589726
Here's the bug for bisecting on individual stories: https://bugs.chromium.org/p/chromium/issues/detail?id=639829
Project Member

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

Mergedinto: 635492
Status: Duplicate (was: Assigned)

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


===== SUSPECTED CL(s) =====
Subject : [system-health] Re-enable full Google Search story on mobile
Author  : petrcermak
Commit description:
  
The original patch adding the story
(https://codereview.chromium.org/2173943003/) was reverted
(https://codereview.chromium.org/2173943003/) due to failures on
Windows. While the problem hasn't been fixed yet
(http://crbug.com/634343), this patch re-enables the story on mobile.

*** NOTE TO SHERIFFS ***
REGULAR CQ:
If this patch causes failures in telemetry_perf_tests, please add the
test name
(benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_mobile.search:portal:google)
to _DISABLED_TESTS in tools/perf/benchmarks/system_health_smoke_test.py
instead of reverting.
PERF CQ:
If this patch causes failures in
system_health.memory_mobile[.reference], please change
SUPPORTED_PLATFORMS in
tools/perf/page_sets/system_health/searching_stories.py to
platforms.NO_PLATFORMS instead of reverting or disabling the benchmark.

BUG=589726,634343

Review-Url: https://codereview.chromium.org/2217113002
Cr-Commit-Position: refs/heads/master@{#410036}
Commit  : 7632abc40c4d27ef688f5a6ced98b2a56e51187e
Date    : Fri Aug 05 12:11:40 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@410033  2714010  6211.89  5  good
chromium@410035  2716467  4486.94  5  good
chromium@410036  2990899  60504.5  5  bad    <--

Bisect job ran on: android_one_perf_bisect
Bug ID: 635491

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: load_search-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/load_search_google
Relative Change: 10.20%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1551
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9003624376138840192


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

| 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!
Labels: SystemHealth-Sheriff

Sign in to add a comment