Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2779523004:
gpu: Bind the mojom::GpuService request on the IO thread. (Closed)
Descriptiongpu: Bind the mojom::GpuService request on the IO thread.
Bind the mojom::GpuService request on the IO thread, so that requests
for memory allocation from the host does not block on the main thread.
Most of the other API implementations of mojom::GpuService interface
needs to happen in the main thread though. So thread hop back onto the
main thread in those cases.
BUG=643746
Review-Url: https://codereview.chromium.org/2779523004
Cr-Commit-Position: refs/heads/master@{#460195}
Committed: https://chromium.googlesource.com/chromium/src/+/0526dc5e7f8cc5b0577b5519893d51a6e87100a1
Patch Set 1 #Patch Set 2 : . #Patch Set 3 : . #
Total comments: 1
Dependent Patchsets: Messages
Total messages: 25 (17 generated)
The CQ bit was checked by sadrul@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by sadrul@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by sadrul@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== gpu: Bind the mojom::GpuService request on the IO thread. ... BUG=643746 ========== to ========== gpu: Bind the mojom::GpuService request on the IO thread. Bind the mojom::GpuService request on the IO thread, so that requests for memory allocation from the host does not block on the main thread. Most of the other API implementations of mojom::GpuService interface needs to happen in the main thread though. So thread hop back onto the main thread in those cases. BUG=643746 ==========
sadrul@chromium.org changed reviewers: + jbauman@chromium.org, rockot@chromium.org
This is a requirement for switching to the mojom api for gpu memory allocation (https://codereview.chromium.org/2701233002/) from chrome. PTAL.
https://codereview.chromium.org/2779523004/diff/40001/services/ui/gpu/gpu_ser... File services/ui/gpu/gpu_service.cc (right): https://codereview.chromium.org/2779523004/diff/40001/services/ui/gpu/gpu_ser... services/ui/gpu/gpu_service.cc:208: FROM_HERE, base::Bind(&GpuService::DestroyGpuMemoryBuffer, weak_ptr_, Just a quick note: I deliberately thread-hopped into the same function, instead of introducing new *OnMain() callbacks, since the function bodies are only a few lines, and I feel it's still pretty easy to follow what's going on (and which part of the function is running on which thread).
rockot@chromium.org changed reviewers: + yzshen@chromium.org
This is unfortunate, but it's hard to imagine a clean API for allowing a single interface binding to split dispatch across multiple task runners. If this comes up more often we might have to consider a more general solution (+yzshen FYI). LGTM in any case.
lgtm
The CQ bit was checked by sadrul@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490731143107780,
"parent_rev": "eb9d3f5f02e186aa3ac173f82da46e018bcb9889", "commit_rev":
"0526dc5e7f8cc5b0577b5519893d51a6e87100a1"}
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490731143107780,
"parent_rev": "eb9d3f5f02e186aa3ac173f82da46e018bcb9889", "commit_rev":
"0526dc5e7f8cc5b0577b5519893d51a6e87100a1"}
Message was sent while issue was closed.
Description was changed from ========== gpu: Bind the mojom::GpuService request on the IO thread. Bind the mojom::GpuService request on the IO thread, so that requests for memory allocation from the host does not block on the main thread. Most of the other API implementations of mojom::GpuService interface needs to happen in the main thread though. So thread hop back onto the main thread in those cases. BUG=643746 ========== to ========== gpu: Bind the mojom::GpuService request on the IO thread. Bind the mojom::GpuService request on the IO thread, so that requests for memory allocation from the host does not block on the main thread. Most of the other API implementations of mojom::GpuService interface needs to happen in the main thread though. So thread hop back onto the main thread in those cases. BUG=643746 Review-Url: https://codereview.chromium.org/2779523004 Cr-Commit-Position: refs/heads/master@{#460195} Committed: https://chromium.googlesource.com/chromium/src/+/0526dc5e7f8cc5b0577b5519893d... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/0526dc5e7f8cc5b0577b5519893d...
Message was sent while issue was closed.
On 2017/03/28 19:09:57, Ken Rockot wrote: > This is unfortunate, but it's hard to imagine a clean API for allowing a single > interface binding to split dispatch across multiple task runners. If this comes > up more often we might have to consider a more general solution (+yzshen FYI). > LGTM in any case. I don't know much about the GpuService interface, can it be split into multiple interfaces?
Message was sent while issue was closed.
On 2017/03/28 22:49:26, yzshen1 wrote: > On 2017/03/28 19:09:57, Ken Rockot wrote: > > This is unfortunate, but it's hard to imagine a clean API for allowing a > single > > interface binding to split dispatch across multiple task runners. If this > comes > > up more often we might have to consider a more general solution (+yzshen FYI). > > LGTM in any case. > > I don't know much about the GpuService interface, can it be split into multiple > interfaces? It wouldn't really make sense. The thread-split is more of an internal detail of the API implementation. As far as the api-users are concerned, they shouldn't need to know (or care) about it. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
