Issue metadata
Sign in to add a comment
|
6.1% regression in angle_perftests/DrawCallPerf_d3d11_null/score on chromium-rel-win7-gpu-nvidia at 420670:420712 |
||||||||||||||||||||||
Issue descriptionPerformance dashboard identified a 6.1% regression in angle_perftests/DrawCallPerf_d3d11_null/score on chromium-rel-win7-gpu-nvidia at revision range 420670:420712. Graph: https://chromeperf.appspot.com/report?masters=ChromiumPerf&bots=chromium-rel-win7-gpu-nvidia&tests=angle_perftests%2FDrawCallPerf_d3d11_null%2Fscore&checked=score%2Cscore_ref%2Cref&rev=420712
,
Sep 29 2016
=== Auto-CCing suspected CL author tedchoc@chromium.org === Hi tedchoc@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 : Ensure CCTs have the proper context menu on tab restore. Author : tedchoc Commit description: Custom tab context menus worked previously because CustomTabActivity was in charge of creating the initial tab, and all subsequent tabs reuse the delegate of the parent tab. When restoring, the TabModelSelectorImpl via TabPersistentStore was creating the tabs and using the default logic in ChromeTabCreator. This change moves ChromeTabCreator creation to the activities themselves, which allows CCTs to overwrite the default tab delegate construction. This also moves the svelte tab saving logic to the selector, which is consistent with all other tab saving behavior (and allows the activity to be unaware of that dependency and enables creation w/ fewer params). BUG=649139 Review-Url: https://codereview.chromium.org/2359923005 Cr-Commit-Position: refs/heads/master@{#420702} Commit : aaaf17110586e60d241acf02148f9be2dc683015 Date : Fri Sep 23 19:49:48 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@420669 30097.4 147.559 27 good chromium@420691 30082.1 148.309 27 good chromium@420697 30033.8 204.475 18 good chromium@420700 30096.8 191.535 12 good chromium@420701 30095.5 99.526 8 good chromium@420702 29587.3 552.0 12 bad <-- chromium@420712 29896.4 271.212 18 bad Bisect job ran on: winx64nvidia_perf_bisect Bug ID: 651101 Test Command: .\src\out\Release_x64\angle_perftests.exe --test-launcher-print-test-stdio=always --test-launcher-jobs=1 Test Metric: DrawCallPerf_d3d11_null/score Relative Change: 1.03% Score: 99.0 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1883 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9000262864991858800 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5910003523780608 | 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!
,
Sep 29 2016
Java only change, so can't impact win. One of the rare times, that I can say with 100% confidence that this was not me.
,
Sep 29 2016
Perf bisect, come on buddy. :( I'll try it again.
,
Sep 29 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9000166051286035136
,
Sep 29 2016
I nudged it forward into a range with two angle rolls.
,
Sep 30 2016
=== Auto-CCing suspected CL author cwallez@chromium.org === Hi cwallez@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 : Implement a separate last row texture unpack buffer upload workaround Author : Corentin Wallez Commit description: When uploading textures from an unpack buffer, some drivers expect an extra row paading, causing them to think the pixel buffer is not big enough. We work around this by uploading the last row separately. BUG= angleproject:1512 Change-Id: I52fb8b35dc450b957f1fafb0b405c81bf0504157 Reviewed-on: https://chromium-review.googlesource.com/385193 Commit-Queue: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit : c5cacd6035ecd0882c2bb483c4ca50380b835c4c Date : Thu Sep 22 14:39:43 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@420712 30054.6 100.912 5 good chromium@420728 30072.8 56.6586 5 good chromium@420730 30117.0 57.2844 5 good chromium@420731 30128.0 40.4166 5 good chromium@420731,angle@44d0a73654 29777.0 203.392 5 good chromium@420731,angle@c5cacd6035 28208.6 189.446 5 bad <-- chromium@420731,angle@f979524a0e 29203.6 59.6766 5 bad chromium@420731,angle@596018ce7b 28765.6 164.19 5 bad chromium@420732 28287.2 124.586 5 bad chromium@420736 28297.2 113.676 5 bad chromium@420744 28240.2 51.9394 5 bad chromium@420776 28258.4 154.204 5 bad Bisect job ran on: winx64nvidia_perf_bisect Bug ID: 651101 Test Command: .\src\out\Release_x64\angle_perftests.exe --test-launcher-print-test-stdio=always --test-launcher-jobs=1 Test Metric: DrawCallPerf_d3d11_null/score Relative Change: 5.98% Score: 99.5 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1885 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9000166051286035136 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=6389488438214656 | 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!
,
Oct 3 2016
Corentin, maybe we can fix this using the caching we discussed in person?
,
Oct 3 2016
I don't see how the CL could be related to this regression because: - Most of the changes are in the GL backend, the regression is for D3D11 - The change to validationES3 is for texImage2D, under pixelUnpackBuffer != nullptr and the test only uses texImage2D once, without pixel unpack buffers. - The only thing left is the change to computeUnpackSize, which is a bugfix (and I don't see how it would get called from DrawCallPerf). On the other hand, the parent commit, 44d0a73654 adds conditions and string compares to DrawCallPerf.
,
Oct 3 2016
Nevermind that last paragraph, it was in DrawBuffersTest instead.
,
Oct 3 2016
It might be the small change to computeRowPitch messed up the optimization somehow. I guess close as WontFix if you can't reproduce this.
,
Oct 3 2016
I was able to reproduce the regression and will make a fix by not computing rowPitch twice when computeUnpackSize is called.
,
Oct 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/0e48719fd39364eae101d5c3c91b943bc501931a commit 0e48719fd39364eae101d5c3c91b943bc501931a Author: Corentin Wallez <cwallez@chromium.org> Date: Mon Oct 03 20:30:38 2016 formatutils: allow reusing rowPitch computation for depthPitch This should fix a null D3D11 backend draw call performance regression. BUG= 651101 Change-Id: I2eb10cddd15f0e7b25b886c89eccd2906e988c72 Reviewed-on: https://chromium-review.googlesource.com/392227 Reviewed-by: Geoff Lang <geofflang@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Corentin Wallez <cwallez@chromium.org> [modify] https://crrev.com/0e48719fd39364eae101d5c3c91b943bc501931a/src/libANGLE/formatutils.h [modify] https://crrev.com/0e48719fd39364eae101d5c3c91b943bc501931a/src/libANGLE/renderer/d3d/d3d11/Image11.cpp [modify] https://crrev.com/0e48719fd39364eae101d5c3c91b943bc501931a/src/libANGLE/renderer/d3d/d3d11/TextureStorage11.cpp [modify] https://crrev.com/0e48719fd39364eae101d5c3c91b943bc501931a/src/libANGLE/formatutils.cpp [modify] https://crrev.com/0e48719fd39364eae101d5c3c91b943bc501931a/src/libANGLE/renderer/d3d/d3d9/Image9.cpp [modify] https://crrev.com/0e48719fd39364eae101d5c3c91b943bc501931a/src/libANGLE/renderer/gl/TextureGL.cpp
,
Oct 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2ae51c4ea3e7b08e143df3a97017519bd3e2ca11 commit 2ae51c4ea3e7b08e143df3a97017519bd3e2ca11 Author: jmadill <jmadill@chromium.org> Date: Tue Oct 04 17:15:21 2016 Roll ANGLE 886de36..4c65524 https://chromium.googlesource.com/angle/angle.git/+log/886de36..4c65524 BUG= chromium:651493 , 651101 , chromium:618464 TBR=geofflang@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2393613002 Cr-Commit-Position: refs/heads/master@{#422837} [modify] https://crrev.com/2ae51c4ea3e7b08e143df3a97017519bd3e2ca11/DEPS
,
Oct 4 2016
Looking at the graph, it seems this fixed half of the regression, but I don't think we'll be able to get back the other half, because computeUnpackSize used to be too simple, and wrong.
,
Oct 5 2016
Great, there's some variance to the tests as well. Thanks for addressing this. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Sep 28 2016