Make GLES2Implementation::SetAggressivelyFreeResources async |
|||
Issue descriptionSetAggressivelyFreeResources does a full finish via GLES2Implementation::FreeEverything. This can cause jank, in particular when there are confounding issues (see https://bugs.chromium.org/p/chromium/issues/list?can=2&q=GLES2Implementation%3A%3AFreeEverything&colspec=ID+Pri+M+Stars+ReleaseBlock+Component+Status+Owner+Summary+OS+Modified&x=m&y=releaseblock&cells=ids ). There are various historical reasons for round-trips, but the main reason left for a full finish is to be able to free shared memory that may be still in-use by the service side. However we should be able to keep references on the service side so that if the client-side frees memory it doesn't cause problem: - if the memory use is ephemeral (e.g. transfer buffers), flush ordering is enough to guarantee that a DestroyTransferBuffer will happen after the service is done with it - if the memory use is more permanent (e.g. queries), but the client is done with it (e.g. query is destroyed), we don't need to wait for the service to be done with it provided it keeps a reference (via gpu::Buffer) - if the memory is still in use by the client, no amount of waiting will help releasing it anyway. Note that if we don't wait for the service to be done with shared memory, it's valid to free it on the client side, but it is not valid to reuse it there any more (i.e. no pooling/recycling of such shared memory).
,
Jul 24 2017
,
Aug 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd8e233ba9ba0e80504ef22e661fecec9d5d71d8 commit bd8e233ba9ba0e80504ef22e661fecec9d5d71d8 Author: Antoine Labour <piman@chromium.org> Date: Thu Aug 24 17:01:31 2017 Add test for SetAggressivelyFreeResources As we refactor underlying logic, we want to make sure we don't regress how much we're freeing. Bug: 741215 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_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 Change-Id: I00e7c5b857223535f399bd93f4813b117fa1390e Reviewed-on: https://chromium-review.googlesource.com/630597 Commit-Queue: Antoine Labour <piman@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/heads/master@{#497090} [modify] https://crrev.com/bd8e233ba9ba0e80504ef22e661fecec9d5d71d8/gpu/BUILD.gn [modify] https://crrev.com/bd8e233ba9ba0e80504ef22e661fecec9d5d71d8/gpu/command_buffer/service/transfer_buffer_manager.h [modify] https://crrev.com/bd8e233ba9ba0e80504ef22e661fecec9d5d71d8/gpu/command_buffer/tests/gl_manager.cc [modify] https://crrev.com/bd8e233ba9ba0e80504ef22e661fecec9d5d71d8/gpu/command_buffer/tests/gl_manager.h [add] https://crrev.com/bd8e233ba9ba0e80504ef22e661fecec9d5d71d8/gpu/command_buffer/tests/gl_set_aggressively_free_resources_unittest.cc
,
Aug 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a9ef39e1415855824d41b62e63d1148d8750934 commit 1a9ef39e1415855824d41b62e63d1148d8750934 Author: Antoine Labour <piman@chromium.org> Date: Thu Aug 24 19:40:33 2017 Allow asynchronously destroying command buffer This refactors the way we allocate/destroy the command buffer to allow the latter to be fully asynchronous: - don't make DestroyTransferBuffer implicitly invalidate the command buffer on the service side (which has a side effect of changing put/get and possibly causing skew in set_get_buffer_count). Instead the client explicitly SetGetBuffer with an invalid buffer. - make sure we Flush (but not Finish) before destroying the transfer buffer. - refactor the client to make sure we don't try to Flush when we don't have a command buffer (and add checks). - allow Finish/WaitFor* to work correctly even if there is no command buffer - some various simplifications/cleanup - added some tests Note: this allows CommandBufferHelper to do free the command buffer asynchronously, but GLES2Implementation::FreeEverything still does a Finish first for other reasons. Follow-up patches will remove that. Bug: 741215 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_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 Change-Id: I075541d55fbb1f0cde018c4f4a588da969d42b98 Reviewed-on: https://chromium-review.googlesource.com/631016 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Commit-Queue: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#497156} [modify] https://crrev.com/1a9ef39e1415855824d41b62e63d1148d8750934/gpu/command_buffer/client/cmd_buffer_helper.cc [modify] https://crrev.com/1a9ef39e1415855824d41b62e63d1148d8750934/gpu/command_buffer/client/cmd_buffer_helper.h [modify] https://crrev.com/1a9ef39e1415855824d41b62e63d1148d8750934/gpu/command_buffer/client/cmd_buffer_helper_test.cc [modify] https://crrev.com/1a9ef39e1415855824d41b62e63d1148d8750934/gpu/command_buffer/client/transfer_buffer_unittest.cc [modify] https://crrev.com/1a9ef39e1415855824d41b62e63d1148d8750934/gpu/command_buffer/service/command_buffer_service.cc [modify] https://crrev.com/1a9ef39e1415855824d41b62e63d1148d8750934/gpu/command_buffer/service/command_buffer_service.h [modify] https://crrev.com/1a9ef39e1415855824d41b62e63d1148d8750934/gpu/command_buffer/service/command_buffer_service_unittest.cc [modify] https://crrev.com/1a9ef39e1415855824d41b62e63d1148d8750934/gpu/ipc/client/command_buffer_proxy_impl.cc [modify] https://crrev.com/1a9ef39e1415855824d41b62e63d1148d8750934/gpu/ipc/client/command_buffer_proxy_impl.h
,
Aug 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e63f4df8b6f344bd33b98201cfbc889f3cca0b13 commit e63f4df8b6f344bd33b98201cfbc889f3cca0b13 Author: Antoine Labour <piman@chromium.org> Date: Thu Aug 24 23:19:31 2017 Allow asynchronously destroying transfer buffer We don't need to Finish provided we Flush before DestroyTransferBuffer, since IPCs are properly ordered. Bug: 741215 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_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 Change-Id: I4b0506de316c3fc3eff67aeba26173f999d42edd Reviewed-on: https://chromium-review.googlesource.com/634215 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Commit-Queue: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#497241} [modify] https://crrev.com/e63f4df8b6f344bd33b98201cfbc889f3cca0b13/gpu/command_buffer/client/ring_buffer.cc [modify] https://crrev.com/e63f4df8b6f344bd33b98201cfbc889f3cca0b13/gpu/command_buffer/client/transfer_buffer.cc [modify] https://crrev.com/e63f4df8b6f344bd33b98201cfbc889f3cca0b13/gpu/command_buffer/client/transfer_buffer_unittest.cc
,
Aug 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e1f40dffde7ac6a629922059d38020fca11b00a commit 9e1f40dffde7ac6a629922059d38020fca11b00a Author: Antoine Labour <piman@chromium.org> Date: Fri Aug 25 17:20:39 2017 Allow asynchronous destroying of mapped memory When a MemoryChunk only contains FREE or FREE_PENDING_TOKEN blocks, we can destroy the whole chunk without blocking. We need to Flush if we have any FREE_PENDING_TOKEN blocks, but otherwise IPC ordering ensures we won't destroy the shared memory on the service side before we're done with it. Bug: 741215 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_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 Change-Id: Ieee9096862d0cd812aa7dbcc1ec9ce80de8220af Reviewed-on: https://chromium-review.googlesource.com/634279 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Commit-Queue: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#497447} [modify] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/BUILD.gn [modify] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/command_buffer/client/cmd_buffer_helper_test.cc [add] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/command_buffer/client/command_buffer_direct_locked.cc [add] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/command_buffer/client/command_buffer_direct_locked.h [modify] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/command_buffer/client/fenced_allocator.cc [modify] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/command_buffer/client/fenced_allocator.h [modify] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/command_buffer/client/fenced_allocator_test.cc [modify] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/command_buffer/client/mapped_memory.cc [modify] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/command_buffer/client/mapped_memory.h [modify] https://crrev.com/9e1f40dffde7ac6a629922059d38020fca11b00a/gpu/command_buffer/client/mapped_memory_unittest.cc
,
Aug 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d2c0c90d62beb476e4aed898a30cdf86709124fb commit d2c0c90d62beb476e4aed898a30cdf86709124fb Author: Antoine Labour <piman@chromium.org> Date: Fri Aug 25 21:06:53 2017 Allow asynchronous destroying of QuerySyncs QuerySyncs are shared memory structures used to receive results of queries. They can only be reused when the query has finished processing on the service side (even if the query was deleted), however it can be deleted after the query was deleted on teh service side. So instead of waiting for results, we can insert a token when freeing a Bucket (a collection of QuerySync within a single shared memory block), even if the bucket has uncompleted, but deleted queries. This patch refactors the QueryTracker and its structures: 1- keep completion tracking data (submit_count) in the QueryInfo rather than the query. That lets us delete the Query as soon as the GLImplementation does 2- correcly mark Queries as pending on EndQuery (instead of BeginQuery) since that's what triggers completion 3- when a QueryInfo is freed (because the corresponding Query was), we can either immediately free (and reuse) its corresponding QuerySync if the query was completed, and otherwise we can keep the QuerySync in per-Bucket "pending" list 4- if all the QuerySyncs in a Bucket are completed, or pending completion, we can free the Bucket (pending token if any query is still pending). Bug: 741215 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_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 Change-Id: I85925b758000b1a0ba7acdf0219223c4b4fd4971 Reviewed-on: https://chromium-review.googlesource.com/634563 Commit-Queue: Antoine Labour <piman@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/heads/master@{#497531} [modify] https://crrev.com/d2c0c90d62beb476e4aed898a30cdf86709124fb/gpu/command_buffer/client/fenced_allocator.cc [modify] https://crrev.com/d2c0c90d62beb476e4aed898a30cdf86709124fb/gpu/command_buffer/client/fenced_allocator.h [modify] https://crrev.com/d2c0c90d62beb476e4aed898a30cdf86709124fb/gpu/command_buffer/client/gles2_implementation.cc [modify] https://crrev.com/d2c0c90d62beb476e4aed898a30cdf86709124fb/gpu/command_buffer/client/gles2_implementation_unittest.cc [modify] https://crrev.com/d2c0c90d62beb476e4aed898a30cdf86709124fb/gpu/command_buffer/client/mapped_memory.cc [modify] https://crrev.com/d2c0c90d62beb476e4aed898a30cdf86709124fb/gpu/command_buffer/client/mapped_memory.h [modify] https://crrev.com/d2c0c90d62beb476e4aed898a30cdf86709124fb/gpu/command_buffer/client/query_tracker.cc [modify] https://crrev.com/d2c0c90d62beb476e4aed898a30cdf86709124fb/gpu/command_buffer/client/query_tracker.h [modify] https://crrev.com/d2c0c90d62beb476e4aed898a30cdf86709124fb/gpu/command_buffer/client/query_tracker_unittest.cc
,
Aug 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f95a74cce904b488721d4ee8f7c946b55d12696 commit 3f95a74cce904b488721d4ee8f7c946b55d12696 Author: Antoine Labour <piman@chromium.org> Date: Fri Aug 25 21:23:12 2017 Remove WaitForCmd in GLES2Implementation::FreeEverything It's no longer needed for correctness, nor to make sure we release as much as possible. Bug: 741215 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_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 Change-Id: Ica02c588e5af7c5e52a86cf036df24775f5f3709 Reviewed-on: https://chromium-review.googlesource.com/634469 Commit-Queue: Antoine Labour <piman@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/heads/master@{#497539} [modify] https://crrev.com/3f95a74cce904b488721d4ee8f7c946b55d12696/gpu/command_buffer/client/gles2_implementation.cc
,
Aug 25 2017
,
Sep 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/325fbc904f562ebb2f5f4942fe315d33bda40041 commit 325fbc904f562ebb2f5f4942fe315d33bda40041 Author: Antoine Labour <piman@chromium.org> Date: Thu Sep 14 22:39:22 2017 Fix SetAggressivelyFreeResourcesTest to work on ES2 If ES3 is not supported, we fall back to ES2 and only test available features. Bug: 741215 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_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 Change-Id: Ied1e4ae7ecd5701f9a050c7eabb0a186f2feb1a5 Reviewed-on: https://chromium-review.googlesource.com/666995 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Commit-Queue: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#502049} [modify] https://crrev.com/325fbc904f562ebb2f5f4942fe315d33bda40041/gpu/command_buffer/tests/gl_set_aggressively_free_resources_unittest.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Jul 13 2017