215ms startup performance regression in gl::ValidateUseProgram() |
|||||
Issue descriptionSuspect CL revision: 44a6fbfd95dd36761ddbf705684683b41cb7c6b1 summary: Optimize resolveLink. author: jmadill@chromium.org This CL is suspected of introducing an average regression of 215ms over the canary population during the first 30 seconds of startup in 71.0.3570.0 canary Windows 64-bit GPU process main thread The regression was detected between versions 71.0.3569.0 and 71.0.3570.0. Execution profile for gl::ValidateUseProgram(): https://uma.googleplex.com/p/chrome/callstacks?sid=eb8e92f6dc0d4940c04c09fb55c750b0
,
Oct 17
This seems to be partially but not fully explained by shifting execution -- in the full first 30 seconds of startup there's a residual 97ms increase in execution time. See the wWinMain delta here: https://uma.googleplex.com/p/chrome/callstacks?sid=9d5b72fd2a53deb92aa91ba00532f6fd. As a side note, the GPU profiling data can be noisy but the difference appears to remain when comparing earlier and later releases so I suspect it's legitimate.
,
Oct 17
The graph seems a bit fishy to me. Inspecting the CL in question: https://chromium-review.googlesource.com/c/angle/angle/+/1255509/7/src/libANGLE/validationES2.cpp#6340 Here we see ValidateUseProgram calls isLinked as long as the Program is valid. https://chromium-review.googlesource.com/c/angle/angle/+/1255509/7/src/libANGLE/Program.cpp#b1280 Here we see the "old" isLinked always calls resolveLink. In the graph I don't see where the "old" resolveLink happens. But the Program's link should be resolved by UseProgram in both cases. Is it possible the resolveLink is being called in the "old" trace before we even get to this flame graph? Or is there some systematic problem with comparing these different stack traces?
,
Oct 17
This is due to a limitation of the flame graph difference display: differences where stacks have completely disappeared between the versions are not visible because they are superimposed "new" version profile which doesn't have those stacks. They're visible in the tree view however: https://uma.googleplex.com/p/chrome/callstacks?sid=78abc37d4d61eeacb156bc4cd78b6cf5 The other issue is that resolveLink() has been inlined, so doesn't appear directly; we only see resolveLinkImpl().
,
Jan 15
,
Jan 15
Looks like this might in fact be related to a change Shabi made: https://chromium-review.googlesource.com/c/1251325 Shabi could you investigate?
,
Yesterday
(38 hours ago)
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/37d2e606c6000985f886a8575526cc3a8cd930b6 commit 37d2e606c6000985f886a8575526cc3a8cd930b6 Author: Shahbaz Youssefi <syoussefi@chromium.org> Date: Mon Jan 21 18:28:56 2019 Make BlobCache use independent from ANGLE_program_cache_control The decision to enable caching or not was based on the size of the cache as directed by eglProgramCacheResizeANGLE, which is incorrect for applications that only use the ANDROID_blob_cache extension. The new behavior is as follows: - If EGL_CONTEXT_PROGRAM_BINARY_CACHE_ENABLED_ANGLE is specifically requested and set to true, but the size of the cache is 0, caching is disabled for the context. - If EGL_CONTEXT_PROGRAM_BINARY_CACHE_ENABLED_ANGLE is specifically requested and set to false, caching is disabled for the context. - Otherwise, caching is left enabled for the context, although the cache possibly has a size of zero, which effectively disables it. During application execution, if the blob cache callbacks are set, the application cache will be used. Bug: 896406 , angleproject:2516 Change-Id: Ic0cabda03fb6bf53994e86e3ede30afc8021d67e Reviewed-on: https://chromium-review.googlesource.com/c/1405708 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org> [modify] https://crrev.com/37d2e606c6000985f886a8575526cc3a8cd930b6/src/libANGLE/Display.cpp [modify] https://crrev.com/37d2e606c6000985f886a8575526cc3a8cd930b6/src/libANGLE/MemoryProgramCache.cpp [modify] https://crrev.com/37d2e606c6000985f886a8575526cc3a8cd930b6/src/libANGLE/BlobCache.cpp [modify] https://crrev.com/37d2e606c6000985f886a8575526cc3a8cd930b6/src/libANGLE/BlobCache.h [modify] https://crrev.com/37d2e606c6000985f886a8575526cc3a8cd930b6/src/tests/egl_tests/EGLBlobCacheTest.cpp
,
Yesterday
(36 hours ago)
,
Yesterday
(36 hours ago)
Thanks Shabi for the quick fix. This should roll into Chrome here: https://chromium-review.googlesource.com/c/chromium/src/+/1425959 We should be able to see when this rolls into Canary. Mike would it be possible to watch the sampling profiler for an improvement in process startup of about 200ms? It seems you were right and the regression was legitimate.
,
Yesterday
(35 hours ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/47cfc8ebe2eb4e984392345d23a82dac81e81452 commit 47cfc8ebe2eb4e984392345d23a82dac81e81452 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Mon Jan 21 21:07:57 2019 Roll src/third_party/angle bf433727853d..c81e7bfeb558 (2 commits) https://chromium.googlesource.com/angle/angle.git/+log/bf433727853d..c81e7bfeb558 git log bf433727853d..c81e7bfeb558 --date=short --no-merges --format='%ad %ae %s' 2019-01-21 syoussefi@chromium.org Vulkan: refactor CommandGraphResource 2019-01-21 syoussefi@chromium.org Make BlobCache use independent from ANGLE_program_cache_control Created with: gclient setdep -r src/third_party/angle@c81e7bfeb558 The AutoRoll server is located here: https://autoroll.skia.org/r/angle-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel BUG= chromium:896406 TBR=syoussefi@chromium.org Change-Id: Ic871aa4ad2f00e369b88cbbda918e48b69ca19aa Reviewed-on: https://chromium-review.googlesource.com/c/1425959 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#624673} [modify] https://crrev.com/47cfc8ebe2eb4e984392345d23a82dac81e81452/DEPS |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by jmad...@chromium.org
, Oct 17Status: WontFix (was: Assigned)