Issue metadata
Sign in to add a comment
|
1.1%-8.9% regression in memory.top_10_mobile at 444491:444541 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jan 19 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8990022760859698432
,
Jan 19 2017
=== Auto-CCing suspected CL author zqzhang@chromium.org === Hi zqzhang@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 : zqzhang Commit : a66fe8713400ed760cd5d78931e536f33c5828d5 Date : Wed Jan 18 22:36:39 2017 Subject: [Media>UI] Make ideal media image size independent of screen density Bisect Details Configuration: android_one_perf_bisect Benchmark : memory.top_10_mobile Metric : memory:chrome:all_processes:reported_by_os:system_memory:java_heap:proportional_resident_size_avg/background/after_http_www_baidu_com_s_word_google Change : 4.59% | 4909568.0 -> 5134677.33333 Revision Result N chromium@444490 4909568 +- 8897.61 6 good chromium@444511 4911445 +- 8019.52 6 good chromium@444517 4917248 +- 11494.4 6 good chromium@444518 5133653 +- 51193.2 6 bad <-- chromium@444519 5119317 +- 12429.4 6 bad chromium@444520 5125120 +- 56977.1 6 bad chromium@444522 5146283 +- 70628.8 6 bad chromium@444532 5134677 +- 62028.8 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-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http.www.baidu.com.s.word.google memory.top_10_mobile Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8990022760859698432 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5065133249789952 | 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!
,
Jan 20 2017
,
Jan 20 2017
Hi zqzhang@: The change has causes a lot (100+) of regressions. Please plan for a quick fix or revert.
,
Jan 20 2017
Yes, the fix is on the way :)
,
Jan 20 2017
,
Jan 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c3a579962e733b26ef12f564732260844d558a56 commit c3a579962e733b26ef12f564732260844d558a56 Author: zqzhang <zqzhang@chromium.org> Date: Fri Jan 20 20:30:58 2017 [Media>UI] Don't scale the icon if it's smaller than the ideal size Previously we scale favicons or media artwork images to the ideal size regardless of their original size. This increases the memory usage as we create another copy of the original image and the size will also be increased if the original image size is smaller than the ideal size. In this CL, if the original image size is smaller than the ideal size, we will reuse the original image and won't scale it anymore. BUG= 682701 Review-Url: https://codereview.chromium.org/2641133004 Cr-Commit-Position: refs/heads/master@{#445147} [modify] https://crrev.com/c3a579962e733b26ef12f564732260844d558a56/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java [modify] https://crrev.com/c3a579962e733b26ef12f564732260844d558a56/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
,
Jan 20 2017
,
Jan 20 2017
Thanks for the quick fix. I will keep the bug open until the perf plots return to original levels.
,
Jan 21 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 23 2017
mustaq@, I've checked the performance dashboard and seems the CL fixed the regression. Can you double-check it? I checked using the following query: https://chromeperf.appspot.com/report?sid=a3423dd42e2ce5431188c1c0bfff1d37818622c9f3f14ab129e5120763946469
,
Jan 23 2017
I skimmed through most of the ~500 regressions, all have recovered. Thanks.
,
Jan 23 2017
Issue 682748 has been merged into this issue.
,
Jan 24 2017
The merge has landed as: https://chromium.googlesource.com/chromium/src.git/+/3c4aff3e70f102ba1f911d060e3c554ac8812f17 [Media>UI] Don't scale the icon if it's smaller than the ideal size Previously we scale favicons or media artwork images to the ideal size regardless of their original size. This increases the memory usage as we create another copy of the original image and the size will also be increased if the original image size is smaller than the ideal size. In this CL, if the original image size is smaller than the ideal size, we will reuse the original image and won't scale it anymore. BUG= 682701 Review-Url: https://codereview.chromium.org/2641133004 Cr-Commit-Position: refs/heads/master@{#445147} (cherry picked from commit c3a579962e733b26ef12f564732260844d558a56) Review-Url: https://codereview.chromium.org/2649173002 . Cr-Commit-Position: refs/branch-heads/2987@{#27} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} Manually changing the merge flags.
,
Mar 14 2017
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by mustaq@chromium.org
, Jan 19 2017