Issue metadata
Sign in to add a comment
|
27.3% regression in angle_perftests at 438149:438200 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Dec 14 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8993289018726690368
,
Dec 14 2016
Corentin, I narrowed this to: Tighten the validation for staying in array buffer bounds BUG=angleproject:1523 Change-Id: I685a9b884e5de73b94900c5b6016ce914b9bfe7f Reviewed-on: https://chromium-review.googlesource.com/418458 Commit-Queue: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org> Do you think there's some optimization we can do here? 27% is a fairly substantial chunk. Can we do some caching somewhere?
,
Dec 14 2016
Ew. There's some gains that could be had by not using CheckedNumeric and do things manually instead. From a profiler run on Cesium I found that a third of the time in ValidateDrawAttribs is in CheckedMul. Other things we could do are: - Assess the robustness of the backends and don't do bounds checking if possible. - Cache the validation in the VertexArray somehow, the tricky part being that the validation doesn't only depend on the VAO state, but also on the maximum vertex and the number of instances.
,
Dec 15 2016
=== PERF REGRESSION === === Auto-CCing suspected CL author cwallez@chromium.org === Hi cwallez@chromium.org, the bisect results pointed to your CL, please take a look at the results. ===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Tighten the validation for staying in array buffer bounds Author : Corentin Wallez Commit description: BUG=angleproject:1523 Change-Id: I685a9b884e5de73b94900c5b6016ce914b9bfe7f Reviewed-on: https://chromium-review.googlesource.com/418458 Commit-Queue: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit : 92db69472bad2344d99b93a470053d73eb13d062 Date : Mon Dec 12 16:54:42 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@438148 190973 4235.32 6 good chromium@438155 187821 6789.66 6 good chromium@438158 188414 5262.94 6 good chromium@438160 189315 5173.27 6 good chromium@438160,angle@f52fe93dca 191646 1916.72 6 good chromium@438160,angle@92db69472b 139471 1226.05 6 bad <-- chromium@438160,angle@a2c7498568 135008 1045.67 6 bad chromium@438160,angle@36fd100d48 140195 3322.49 6 bad chromium@438161 136921 1545.63 6 bad chromium@438174 137106 883.272 6 bad chromium@438200 136542 1253.42 6 bad Bisect job ran on: winx64nvidia_perf_bisect Bug ID: 674143 Test Command: .\src\out\Release_x64\angle_perftests.exe --test-launcher-print-test-stdio=always --test-launcher-jobs=1 Test Metric: DrawCallPerf_gl_null/score Relative Change: 28.50% Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1977 Job details: https://chromeperf.appspot.com/buildbucket_job_status/8993289018726690368 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5809052234285056 | 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!
,
Dec 19 2016
27% regression is unacceptable. I'm going to try and reduce this.
,
Dec 19 2016
Corentin said he's working on this.
,
Dec 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/71168a07faaadc6896d8a634fd8862d8709a58c9 commit 71168a07faaadc6896d8a634fd8862d8709a58c9 Author: Corentin Wallez <cwallez@chromium.org> Date: Mon Dec 19 23:11:18 2016 Manually write overflow checks for glDraw* The usage of checked numerics showed has a big hotspot in ValidateDrawAttribs. BUG= angleproject:1671 BUG= chromium:674143 Change-Id: I96392e099b2257e465fb47e1c96f9aa1f54a89ba Reviewed-on: https://chromium-review.googlesource.com/422428 Commit-Queue: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org> [modify] https://crrev.com/71168a07faaadc6896d8a634fd8862d8709a58c9/src/libANGLE/validationES.cpp
,
Dec 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/47cd7960532e2a1bd4e7a987b69c66f632f7f6c9 commit 47cd7960532e2a1bd4e7a987b69c66f632f7f6c9 Author: jmadill <jmadill@chromium.org> Date: Tue Dec 20 19:12:22 2016 Roll ANGLE f047097..71168a0 https://chromium.googlesource.com/angle/angle.git/+log/f047097..71168a0 BUG= chromium:674143 TBR=cwallez@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/2597443002 Cr-Commit-Position: refs/heads/master@{#439866} [modify] https://crrev.com/47cd7960532e2a1bd4e7a987b69c66f632f7f6c9/DEPS
,
Dec 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67a71ece49fdbe3ef686ea54ded42f9523f418e5 commit 67a71ece49fdbe3ef686ea54ded42f9523f418e5 Author: jam <jam@chromium.org> Date: Tue Dec 20 20:12:50 2016 Revert of Roll ANGLE f047097..71168a0 (patchset #1 id:1 of https://codereview.chromium.org/2597443002/ ) Reason for revert: https://build.chromium.org/p/chromium/builders/Win%20x64/builds/6994/steps/compile/logs/stdio ninja -t msvc -e environment.x64 -- C:\b\c\cipd\goma/gomacc.exe "C:\b\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\VC\bin\amd64/cl.exe" /nologo /showIncludes /FC @obj/third_party/angle/src/tests/angle_perftests/UniformsPerf.obj.rsp /c ../../third_party/angle/src/tests/perf_tests/UniformsPerf.cpp /Foobj/third_party/angle/src/tests/angle_perftests/UniformsPerf.obj /Fd"obj/third_party/angle/src/tests/angle_perftests_cc.pdb" c:\b\c\b\win_x64_archive\src\third_party\angle\src\tests\perf_tests\uniformsperf.cpp(148): error C2220: warning treated as error - no 'object' file generated c:\b\c\b\win_x64_archive\src\third_party\angle\src\tests\perf_tests\uniformsperf.cpp(148): warning C4267: 'initializing': conversion from 'size_t' to 'GLint', possible loss of data c:\b\c\b\win_x64_archive\src\third_party\angle\src\tests\perf_tests\uniformsperf.cpp(149): warning C4267: 'initializing': conversion from 'size_t' to 'GLint', possible loss of data Original issue's description: > Roll ANGLE f047097..71168a0 > > https://chromium.googlesource.com/angle/angle.git/+log/f047097..71168a0 > > BUG= chromium:674143 > > TBR=cwallez@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 > > Committed: https://crrev.com/47cd7960532e2a1bd4e7a987b69c66f632f7f6c9 > Cr-Commit-Position: refs/heads/master@{#439866} TBR=cwallez@chromium.org,jmadill@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= chromium:674143 Review-Url: https://codereview.chromium.org/2587413003 Cr-Commit-Position: refs/heads/master@{#439872} [modify] https://crrev.com/67a71ece49fdbe3ef686ea54ded42f9523f418e5/DEPS
,
Dec 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b5de482ad9cc882bf3ef9a73f3d75953a3350c44 commit b5de482ad9cc882bf3ef9a73f3d75953a3350c44 Author: jmadill <jmadill@chromium.org> Date: Tue Dec 20 23:15:24 2016 Roll ANGLE f047097..d901983 https://chromium.googlesource.com/angle/angle.git/+log/f047097..d901983 BUG=None,chromium:668223,chromium:674143 TBR=cwallez@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/2589343002 Cr-Commit-Position: refs/heads/master@{#439916} [modify] https://crrev.com/b5de482ad9cc882bf3ef9a73f3d75953a3350c44/DEPS
,
Dec 21 2016
Excellent work Corentin. The test seems to have recovered.
,
Dec 21 2016
Thanks! |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by jmad...@chromium.org
, Dec 14 2016