Issue metadata
Sign in to add a comment
|
74.7% regression in blink_perf.canvas at 399453:399464 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jun 14 2016
=== Auto-CCing suspected CL author msarett@google.com === Hi msarett@google.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 : Update libpng to 1.6.22 Author : msarett Commit description: BUG= 599917 BUG= 618061 Review-Url: https://codereview.chromium.org/2021403002 Cr-Commit-Position: refs/heads/master@{#399464} Commit : 8c0ac8d882561cca77d5184248708b0ce9c851d4 Date : Mon Jun 13 16:19:43 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@399452 212.893 1.28144 5 good chromium@399458 212.105 3.13468 5 good chromium@399461 212.178 2.3473 5 good chromium@399463 215.781 2.14336 5 good chromium@399464 364.292 9.57979 5 bad <-- Bisect job ran on: mac_retina_perf_bisect Bug ID: 619850 Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.canvas Test Metric: toBlob_duration/toBlob_duration Relative Change: 71.12% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_bisect/builds/1332 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9009894376284668672 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5772423689928704 | 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!
,
Jun 14 2016
===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Update libpng to 1.6.22 Author : msarett Commit description: BUG= 599917 BUG= 618061 Review-Url: https://codereview.chromium.org/2021403002 Cr-Commit-Position: refs/heads/master@{#399464} Commit : 8c0ac8d882561cca77d5184248708b0ce9c851d4 Date : Mon Jun 13 16:19:43 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@399452 212.9 2.80482 5 good chromium@399458 210.237 2.15102 5 good chromium@399461 211.876 0.823958 5 good chromium@399463 214.138 1.93407 5 good chromium@399464 366.748 3.8223 5 bad <-- Bisect job ran on: mac_retina_perf_bisect Bug ID: 619850 Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.canvas Test Metric: toBlob_duration/toBlob_duration Relative Change: 72.26% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_bisect/builds/1333 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9009894371754975904 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5874606330609664 | 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!
,
Jun 16 2016
This is a performance regression in png encoding. I've identified the cause and proposed a fix to upstream. https://github.com/glennrp/libpng/pull/116
,
Jun 20 2016
Patch landed in upstream as https://github.com/glennrp/libpng/commit/9c04f57cabbf736e91b831858d0eeecca703eabf
,
Jun 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b43ad19eed3d9b660f2fc3e445e250a1b42b6b93 commit b43ad19eed3d9b660f2fc3e445e250a1b42b6b93 Author: msarett <msarett@google.com> Date: Wed Jun 22 16:14:08 2016 Fix performance regression in png encoding caused by libpng update libpng calculates "filter heuristics" to choose which png filter to apply. Chrome need not calculate these "filter heuristics" because we always choose to use the SUB filter, for speed. libpng 1.6 refactors the SUB filter to share code, also forcing us to calculate the filter heuristics, even though we don't need to. This caused a performance regression. This CL cherry picks a fix from upstream that: (1) Passes PNG_SIZE_MAX to the filter code. This allows the compiler to optimize out the heuristic calculation code, fixing the performance regression. (2) Fixes overflow handling in the calculation of filter heuristics. This won't affect Chrome. BUG= 619850 BUG= 599917 Review-Url: https://codereview.chromium.org/2062423002 Cr-Commit-Position: refs/heads/master@{#401298} [modify] https://crrev.com/b43ad19eed3d9b660f2fc3e445e250a1b42b6b93/third_party/WebKit/Source/platform/image-encoders/PNGImageEncoder.cpp [modify] https://crrev.com/b43ad19eed3d9b660f2fc3e445e250a1b42b6b93/third_party/libpng/BUILD.gn [modify] https://crrev.com/b43ad19eed3d9b660f2fc3e445e250a1b42b6b93/third_party/libpng/README.chromium [modify] https://crrev.com/b43ad19eed3d9b660f2fc3e445e250a1b42b6b93/third_party/libpng/libpng.gyp [modify] https://crrev.com/b43ad19eed3d9b660f2fc3e445e250a1b42b6b93/third_party/libpng/pngwutil.c
,
Jun 23 2016
Mac regressions are fixed. Expecting the same for Win/Android once results are available.
,
Jun 27 2016
The fix isn't working on all platforms. I suspect it is compiler related, given that the fix depends on the compiler making optimizations. Mac/Linux clang is fine, but Nexus 5x (GCC I think?) and Windows (maybe MSVC?) are not fixed. I've reached back out to upstream about landing a more portable fix.
,
Jul 5 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67058eff9c2a51e589388d7d494b1490530a1ec5 commit 67058eff9c2a51e589388d7d494b1490530a1ec5 Author: msarett <msarett@google.com> Date: Fri Jul 08 14:29:14 2016 Fix for png encoding performance regressions - downstream diff The original fix was: > Fix performance regression in png encoding caused by libpng update > > libpng calculates "filter heuristics" to choose which png > filter to apply. Chrome need not calculate these "filter > heuristics" because we always choose to use the SUB filter, > for speed. > > libpng 1.6 refactors the SUB filter to share code, also > forcing us to calculate the filter heuristics, even though > we don't need to. This caused a performance regression. > > This CL cherry picks a fix from upstream that: > (1) Passes PNG_SIZE_MAX to the filter code. This allows > the compiler to optimize out the heuristic calculation > code, fixing the performance regression. > (2) Fixes overflow handling in the calculation of filter > heuristics. This won't affect Chrome. > > Review-Url: https://codereview.chromium.org/2062423002 This fix was effective everywhere we use clang, but did not work on Android (GCC) or Windows (MSVS) because the compiler was not applying the appropriate optimizations. This CL contains a more portable fix. This was briefly landed by libpng, but backed out. They fear it increases code size and that the compiler can be forced to make the same optimization. FWIW, we have demonstrated that we can force the compiler to make the same optimization. https://codereview.chromium.org/2119813003/ But we feel that changing the compiler options is fragile and it is better to just carry a diff. BUG= 619850 BUG= 599917 Review-Url: https://codereview.chromium.org/2132783002 Cr-Commit-Position: refs/heads/master@{#404379} [modify] https://crrev.com/67058eff9c2a51e589388d7d494b1490530a1ec5/third_party/libpng/README.chromium [modify] https://crrev.com/67058eff9c2a51e589388d7d494b1490530a1ec5/third_party/libpng/pngwutil.c
,
Jul 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38bef90282b731572ca2fab9efaef54d7e3b3726 commit 38bef90282b731572ca2fab9efaef54d7e3b3726 Author: msarett <msarett@google.com> Date: Mon Jul 11 14:53:49 2016 Update README.chromium in libpng BUG= 599917 BUG= 619850 Review-Url: https://codereview.chromium.org/2138563002 Cr-Commit-Position: refs/heads/master@{#404658} [modify] https://crrev.com/38bef90282b731572ca2fab9efaef54d7e3b3726/third_party/libpng/README.chromium
,
Jul 12 2016
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by oth@chromium.org
, Jun 14 2016