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

Issue 635801 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
OOO until Feb 4th
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

8.8%-33.8% regression in top_10_mobile_memory at 410333:410343

Project Member Reported by rmcilroy@chromium.org, Aug 9 2016

Issue description

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

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

Hi wnwen@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 : Reland of Separate deferred startup into tasks.
Author  : wnwen
Commit description:
  
Original: https://codereview.chromium.org/2121863002/

Fixes:
- Guarantee ChromeApplication#onStartWithNative has been called by
  custom tabs before posting deferred startup.

BUG= 614452 
TBR=dtrainor@chromium.org,agrieve@chromium.org,asvitkine@chromium.org,pasko@chromium.org

Review-Url: https://codereview.chromium.org/2207933002
Cr-Commit-Position: refs/heads/master@{#410342}
Commit  : bcdc694e5fb4cf003124cf5b940e4807322a184e
Date    : Mon Aug 08 13:53:49 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@410332  7112278  592453   5  good
chromium@410338  7475539  539765   8  good
chromium@410341  7573087  432111   8  good
chromium@410342  8993385  448561   8  bad    <--
chromium@410343  9561980  732577   5  bad

Bisect job ran on: android_nexus7_perf_bisect
Bug ID: 635801

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests top_10_mobile_memory
Test Metric: memory:chrome:all_processes:reported_by_chrome:v8:effective_size_avg/http___search.yahoo.com_search;_ylt_?p_google
Relative Change: 34.44%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3172
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9004819618972234960


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

| 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 4 by wnwen@chromium.org, Aug 9 2016

I find it hard to believe that my pure java-side change affected v8 memory size. I'll dig in a bit and see if I can uncover anything suspicious.

Other interesting CLs include v8 rolls and changes to this benchmark (memory_top10_mobile): https://codereview.chromium.org/2123713006

https://chromium.googlesource.com/chromium/src/+log/abd0cd6d855eaf34c9aa05aa1ccbe27cf09815b1%5E..03e7cd75500ae29a36da17b854eea4f42c93186a?pretty=fuller
Cc: perezju@chromium.org
Owner: rmcilroy@chromium.org
That does seem unlikely. perezju@ it seems more likely that your change to the memory_top10_mobile benchmark in https://codereview.chromium.org/2123713006 caused this, could that be the case?

In the meantime, I'll kick off another benchmark.
Cc: rmcilroy@chromium.org
Owner: perezju@chromium.org
Yes. Certainly I would suspect that my CL could have caused this.

I'll investigate some further, but probably it's just some artificial change due to the way we measure things and no real regression.

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


===== SUSPECTED CL(s) =====
Subject : Reland of Separate deferred startup into tasks.
Author  : wnwen
Commit description:
  
Original: https://codereview.chromium.org/2121863002/

Fixes:
- Guarantee ChromeApplication#onStartWithNative has been called by
  custom tabs before posting deferred startup.

BUG= 614452 
TBR=dtrainor@chromium.org,agrieve@chromium.org,asvitkine@chromium.org,pasko@chromium.org

Review-Url: https://codereview.chromium.org/2207933002
Cr-Commit-Position: refs/heads/master@{#410342}
Commit  : bcdc694e5fb4cf003124cf5b940e4807322a184e
Date    : Mon Aug 08 13:53:49 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@410332  3110302  137529   5  good
chromium@410338  3241106  158126   5  good
chromium@410341  3062849  121156   5  good
chromium@410342  3689023  237064   5  bad    <--
chromium@410343  3743126  236881   5  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 635801

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests top_10_mobile_memory
Test Metric: memory:chrome:all_processes:reported_by_chrome:v8:effective_size_avg/http___m.intl.taobao.com_group-purchase.html
Relative Change: 20.35%
Score: 99.5

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


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

| 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!
It's strange the bisect bot seems convinced that this is wnwen's change. Could the startup changes have effected when the memory measurements are taken?
Cc: mythria@chromium.org petrcermak@chromium.org
Owner: wnwen@chromium.org
Wait, I've just noted that this is about some "top_10_mobile_memory" v8 benchmark [1], and not memory.top_10_mobile. So my CL could not have affected this. And it doesn't, as it falls within:

  chromium@410332  3110302  137529   5  good
  chromium@410338  3241106  158126   5  good

wnwen, could you run a trybot with a revert of your CL to see the effect it has?

Should be something like

$ src/tools/perf/run_benchmark try android-nexus5 top_10_mobile_memory

[1]: https://cs.chromium.org/chromium/src/tools/perf/benchmarks/v8.py?type=cs&q=top_10_mobile_memory&sq=package:chromium

+mythria, could you rename these benchmarks to something more descriptive like, maybe, v8.top_10_mobile_memory?

Comment 11 by wnwen@chromium.org, Aug 10 2016

Will do. Yes, it is possible that reordering startup and running it earlier on idle could have affected when the measurements were taken. Unfortunately I'm not familiar with these metrics nor their python scripts, so it might take a while. :/

Comment 12 by wnwen@chromium.org, Aug 10 2016

It's also possible that this increase is normal and expected.

We now complete deferred startup tasks at least 1 second earlier when idle, which in turn run warm up tasks and improve the user experience. Of course, this also depends on how the benchmark is actually measured, which I'm trying to figure out but not sure.

Comment 13 by wnwen@chromium.org, Aug 11 2016

Here are the perf try jobs results:

With my CL reverted: http://storage.googleapis.com/chromium-telemetry/html-results/results-2016-08-10_13-07-45

HEAD (no revert): http://storage.googleapis.com/chromium-telemetry/html-results/results-2016-08-11_04-46-02

Not sure why, but many stats decrease/increase significantly in both cases. I think it's a result of our start-up timing being changed (earlier and faster deferred tasks completed for users), which in turn means the metrics are changed, but it's just a new baseline, there's no code regression.
Perf sheriff ping: reminder to follow up on possible performance issues
Status: WontFix (was: Assigned)
It looks like this is just shifting of metrics due to changes in measurement times. Closing as won't fix.

Sign in to add a comment