Issue metadata
Sign in to add a comment
|
33.1% regression in kraken at 475746:475839 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jun 2 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8977855782090640816
,
Jun 3 2017
=== Auto-CCing suspected CL author mtklein@chromium.org === Hi mtklein@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 : Mike Klein Commit : b5e4842543318e1eac827433855c9a37cdb7f26a Date : Wed May 31 02:40:10 2017 Subject: clean up now that min_stride == 1 Bisect Details Configuration: linux_perf_bisect Benchmark : kraken Metric : Total/Total Change : 33.49% | 1033.43333333 -> 1379.48333333 Revision Result N chromium@475745 1033.43 +- 13.39 6 good chromium@475792 1036.08 +- 17.998 6 good chromium@475804 1039.7 +- 13.6821 6 good chromium@475810 1036.9 +- 11.8651 6 good chromium@475811 1039.13 +- 18.8035 6 good chromium@475811,skia@7f6ad01336 1038.43 +- 12.3666 6 good chromium@475811,skia@b5e4842543 1386.5 +- 12.7805 6 bad <-- chromium@475812 1385.55 +- 14.086 6 bad chromium@475813 1380.48 +- 14.7468 6 bad chromium@475816 1383.4 +- 9.05649 6 bad chromium@475839 1379.48 +- 10.221 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests kraken Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8977855782090640816 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=4632732195356672 | 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 Speed>Bisection. Thank you!
,
Jun 3 2017
Issue 729239 has been merged into this issue.
,
Jun 3 2017
Issue 729240 has been merged into this issue.
,
Jun 4 2017
It's a surprise that this CL caused a performance regression, but we also saw a similar confusing change in our local performance tests. I'll look into this this week.
,
Jun 5 2017
The following revision refers to this bug: https://skia.googlesource.com/skia/+/9beafc41afa3c78ffa12649dcde73628c277da9c commit 9beafc41afa3c78ffa12649dcde73628c277da9c Author: Mike Klein <mtklein@chromium.org> Date: Mon Jun 05 13:46:40 2017 have start_pipeline() return limit again This is spooky. I don't quite yet understand why, but this makes things much faster. Performance regressed across the board when we no longer needed the value and changed it to return void: https://perf.skia.org/e/?begin=1496176469&keys=6994&xbaroffset=28513 You can see similar regressions following this Chromium bug link. BUG= chromium:729237 Change-Id: I68371b0456014f909acf819aca52aa4f4f187460 Reviewed-on: https://skia-review.googlesource.com/18580 Reviewed-by: Herb Derby <herb@google.com> Reviewed-by: Mike Reed <reed@google.com> Commit-Queue: Mike Klein <mtklein@chromium.org> [modify] https://crrev.com/9beafc41afa3c78ffa12649dcde73628c277da9c/src/core/SkBlitter.cpp [modify] https://crrev.com/9beafc41afa3c78ffa12649dcde73628c277da9c/src/jumper/SkJumper_generated_win.S [modify] https://crrev.com/9beafc41afa3c78ffa12649dcde73628c277da9c/src/jumper/SkJumper_stages_lowp.cpp [modify] https://crrev.com/9beafc41afa3c78ffa12649dcde73628c277da9c/src/jumper/SkJumper_generated.S [modify] https://crrev.com/9beafc41afa3c78ffa12649dcde73628c277da9c/src/jumper/SkJumper_stages.cpp [modify] https://crrev.com/9beafc41afa3c78ffa12649dcde73628c277da9c/src/core/SkRasterPipeline.h [modify] https://crrev.com/9beafc41afa3c78ffa12649dcde73628c277da9c/src/jumper/SkJumper.cpp
,
Jun 5 2017
The following revision refers to this bug: https://skia.googlesource.com/skia/+/eeccbf735deedb88313c480dde29bfd8193ee5f3 commit eeccbf735deedb88313c480dde29bfd8193ee5f3 Author: Mike Klein <mtklein@chromium.org> Date: Mon Jun 05 15:29:11 2017 Real fix for stage perf regression. When we made start_pipeline() return void, the call into the tail!=0 run of the pipeline became eligble to be a tail-call, and Clang made that choice. This had the side effect of not going through vzeroupper on those tails. We now mark start_pipeline() as inelligible for tail calls when targeting AVX+. All paths go through the vzeroupper at the end. BUG= chromium:729237 Change-Id: I2099931284214f24c67b38979b3ad4b4d10e8bba Reviewed-on: https://skia-review.googlesource.com/18591 Reviewed-by: Herb Derby <herb@google.com> Commit-Queue: Mike Klein <mtklein@chromium.org> [modify] https://crrev.com/eeccbf735deedb88313c480dde29bfd8193ee5f3/src/jumper/SkJumper_generated.S [modify] https://crrev.com/eeccbf735deedb88313c480dde29bfd8193ee5f3/src/jumper/SkJumper_stages.cpp [modify] https://crrev.com/eeccbf735deedb88313c480dde29bfd8193ee5f3/src/jumper/SkJumper_generated_win.S [modify] https://crrev.com/eeccbf735deedb88313c480dde29bfd8193ee5f3/src/core/SkRasterPipeline.h [modify] https://crrev.com/eeccbf735deedb88313c480dde29bfd8193ee5f3/src/jumper/SkJumper.cpp
,
Jun 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9b2a580a443da7e5f83004dffa68f6db904648bb commit 9b2a580a443da7e5f83004dffa68f6db904648bb Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org> Date: Mon Jun 05 17:08:48 2017 Roll src/third_party/skia/ c8167f167..eee4d6e4e (4 commits) https://skia.googlesource.com/skia.git/+log/c8167f167dc7..eee4d6e4e8cc $ git log c8167f167..eee4d6e4e --date=short --no-merges --format='%ad %ae %s' 2017-06-05 robertphillips Make instantiate return a Boolean 2017-06-05 mtklein have start_pipeline() return limit again 2017-06-05 mtklein implement SkRasterPipelineBlitter::blitAntiH2() 2017-06-05 robertphillips Re-enable single channel renderability for ANGLE ES2 Created with: roll-dep src/third_party/skia BUG= 729237 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=allanmac@chromium.org Change-Id: Ic401d03d33735b8efdc745f0c27d4e8543bf7ef0 Reviewed-on: https://chromium-review.googlesource.com/523805 Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org> Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#477004} [modify] https://crrev.com/9b2a580a443da7e5f83004dffa68f6db904648bb/DEPS
,
Jun 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4dcf5cd233ba345656f44c8c7003cbc32005d46e commit 4dcf5cd233ba345656f44c8c7003cbc32005d46e Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org> Date: Mon Jun 05 19:01:21 2017 Roll src/third_party/skia/ eee4d6e4e..0e022297f (16 commits) https://skia.googlesource.com/skia.git/+log/eee4d6e4e8cc..0e022297fee8 $ git log eee4d6e4e..0e022297f --date=short --no-merges --format='%ad %ae %s' 2017-06-05 mtklein add color 2017-06-05 bsalomon Make GrSimpleMeshDrawOpHelper record whether coverage can be implemented as alpha 2017-06-05 robertphillips Destroy ANGLE displays in destroyGLContext 2017-06-05 mtklein lowp: add move_src_dst and srcover 2017-06-04 mtklein slight streamlining for lowp load_8888 with pshufb 2017-06-05 egdaniel Remove blacklist of unit tests on Adreno Vulkan 2017-06-05 mtklein Exclude SkJumper_stages_lowp.cpp from G3 build. 2017-06-05 mtklein Real fix for stage perf regression. 2017-06-05 brianosman Simplify some Viewer code, and fix a few bugs 2017-06-05 msarett SkImage::makeColorSpace(): Fix nullptr->sRGB bug with picture images 2017-06-01 scroggo Replace BMP calls to new with calls to malloc 2017-06-05 egdaniel Remove hardstop blacklist for nexus player vulkan 2017-06-05 liyuqian A little help message for Android viewer's build dir 2017-05-22 cathy.bao Handle different types of streams in different jpeg source managers 2017-06-05 scroggo Support passing single SKP to get_images_from_skps 2017-06-05 reed add testing flag to force rasterpipeline Created with: roll-dep src/third_party/skia BUG= 729237 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=allanmac@chromium.org Change-Id: Ie6ee8e09f2454d15c898831e65896e1bfbd6b908 Reviewed-on: https://chromium-review.googlesource.com/523666 Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org> Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#477047} [modify] https://crrev.com/4dcf5cd233ba345656f44c8c7003cbc32005d46e/DEPS
,
Jun 5 2017
,
Jun 7 2017
Looks like that did the trick. The relevant changes from above are: 2017-06-05 mtklein have start_pipeline() return limit again (bandaid) 2017-06-05 mtklein Real fix for stage perf regression. (long-term fix) |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by m...@chromium.org
, Jun 2 2017