Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2064853002:
Add timeout to swap fence (Closed)
DescriptionAdd timeout to swap fence
In some cases (mainly on Mac OS 10.9), it appears that a glFence
may never pass. To work around this issue (and make the code more
robust in similar cases), a 32ms timeout has been added, after which
we will just call glFinish rather than continuing to wait on the fence.
Will add an UMA stat in a follow-up CL to make sure this is very rare
on 10.9 - if this turns out to be at all common, we should probably
modify 10.9 to indicate that it doesn't support glFence.
BUG=618075
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
Committed: https://crrev.com/d84042a6b9b061fd319c0068bdd0c786a16fcc85
Cr-Commit-Position: refs/heads/master@{#399598}
Patch Set 1 #
Total comments: 4
Patch Set 2 : feedback #Messages
Total messages: 15 (8 generated)
Description was changed from ========== Add timeout to swap fence In some cases (especially on Mac OS 10.9), it appears that a glFence may never pass. To work around this issue (and make the code more robust in similar cases, a 32ms timeout has been added, after which we will just call glFinish rather than continuing to wait on the fence. BUG=618075 ========== to ========== Add timeout to swap fence In some cases (especially on Mac OS 10.9), it appears that a glFence may never pass. To work around this issue (and make the code more robust in similar cases, a 32ms timeout has been added, after which we will just call glFinish rather than continuing to wait on the fence. BUG=618075 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Description was changed from ========== Add timeout to swap fence In some cases (especially on Mac OS 10.9), it appears that a glFence may never pass. To work around this issue (and make the code more robust in similar cases, a 32ms timeout has been added, after which we will just call glFinish rather than continuing to wait on the fence. BUG=618075 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Add timeout to swap fence In some cases (mainly on Mac OS 10.9), it appears that a glFence may never pass. To work around this issue (and make the code more robust in similar cases), a 32ms timeout has been added, after which we will just call glFinish rather than continuing to wait on the fence. Will add an UMA stat in a follow-up CL to make sure this is very rare on 10.9 - if this turns out to be at all common, we should probably modify 10.9 to indicate that it doesn't support glFence. BUG=618075 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
ericrk@chromium.org changed reviewers: + ccameron@chromium.org
The CQ bit was checked by ericrk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2064853002/1
lgtm. Would be nice to include a crbug ref for the issue, since it's pretty terrifying. https://codereview.chromium.org/2064853002/diff/1/gpu/ipc/service/image_trans... File gpu/ipc/service/image_transport_surface_overlay_mac.mm (right): https://codereview.chromium.org/2064853002/diff/1/gpu/ipc/service/image_trans... gpu/ipc/service/image_transport_surface_overlay_mac.mm:186: // fences appear to sometimes get lost and will naver pass. Add a 32ms s/naver/never https://codereview.chromium.org/2064853002/diff/1/gpu/ipc/service/image_trans... gpu/ipc/service/image_transport_surface_overlay_mac.mm:188: int timeout = 32; s/timeout/timeout_msec
The CQ bit was checked by ericrk@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ccameron@chromium.org Link to the patchset: https://codereview.chromium.org/2064853002/#ps20001 (title: "feedback")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2064853002/20001
https://codereview.chromium.org/2064853002/diff/1/gpu/ipc/service/image_trans... File gpu/ipc/service/image_transport_surface_overlay_mac.mm (right): https://codereview.chromium.org/2064853002/diff/1/gpu/ipc/service/image_trans... gpu/ipc/service/image_transport_surface_overlay_mac.mm:186: // fences appear to sometimes get lost and will naver pass. Add a 32ms On 2016/06/13 22:16:14, ccameron wrote: > s/naver/never Done. https://codereview.chromium.org/2064853002/diff/1/gpu/ipc/service/image_trans... gpu/ipc/service/image_transport_surface_overlay_mac.mm:188: int timeout = 32; On 2016/06/13 22:16:14, ccameron wrote: > s/timeout/timeout_msec Done.
Message was sent while issue was closed.
Description was changed from ========== Add timeout to swap fence In some cases (mainly on Mac OS 10.9), it appears that a glFence may never pass. To work around this issue (and make the code more robust in similar cases), a 32ms timeout has been added, after which we will just call glFinish rather than continuing to wait on the fence. Will add an UMA stat in a follow-up CL to make sure this is very rare on 10.9 - if this turns out to be at all common, we should probably modify 10.9 to indicate that it doesn't support glFence. BUG=618075 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Add timeout to swap fence In some cases (mainly on Mac OS 10.9), it appears that a glFence may never pass. To work around this issue (and make the code more robust in similar cases), a 32ms timeout has been added, after which we will just call glFinish rather than continuing to wait on the fence. Will add an UMA stat in a follow-up CL to make sure this is very rare on 10.9 - if this turns out to be at all common, we should probably modify 10.9 to indicate that it doesn't support glFence. BUG=618075 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== Add timeout to swap fence In some cases (mainly on Mac OS 10.9), it appears that a glFence may never pass. To work around this issue (and make the code more robust in similar cases), a 32ms timeout has been added, after which we will just call glFinish rather than continuing to wait on the fence. Will add an UMA stat in a follow-up CL to make sure this is very rare on 10.9 - if this turns out to be at all common, we should probably modify 10.9 to indicate that it doesn't support glFence. BUG=618075 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Add timeout to swap fence In some cases (mainly on Mac OS 10.9), it appears that a glFence may never pass. To work around this issue (and make the code more robust in similar cases), a 32ms timeout has been added, after which we will just call glFinish rather than continuing to wait on the fence. Will add an UMA stat in a follow-up CL to make sure this is very rare on 10.9 - if this turns out to be at all common, we should probably modify 10.9 to indicate that it doesn't support glFence. BUG=618075 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/d84042a6b9b061fd319c0068bdd0c786a16fcc85 Cr-Commit-Position: refs/heads/master@{#399598} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/d84042a6b9b061fd319c0068bdd0c786a16fcc85 Cr-Commit-Position: refs/heads/master@{#399598} |
