gpu: Add GpuFence framework.

gpu: Add GpuFence framework.

This introduces a new type of synchronization primitive that can be
used to more effectively synchronize access to resources shared with
the GPU. It's useful when the resources dealing with resources that
are used in multiple processes or accessed both using the GPU and the
CPU.
    
It provides the building blocks needed to effectively support
explicit synchronization and reduce the synchronization overhead
for GpuMemoryBuffers on platforms without native buffer support.
    
A software implementation of the GpuFence interface is provided as
part of this patch. This software implementation can be used as
a fallback on platforms without native sync primitives and is
ideal for synchronizing access to GpuMemoryBuffers when backed by
shared memory.
    
BUG=653908
TEST=gfx_unittests, gl_tests
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

[reveman, 2016-09-30 11:24:24]

Description was changed from

==========
gpu: GpuFence prototype.

BUG=
==========

to

==========
gpu: GpuFence prototype.

BUG=
CQ_INCLUDE_TRYBOTS=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
==========

[reveman, 2016-09-30 11:24:25]

Description was changed from

==========
gpu: GpuFence prototype.

BUG=
CQ_INCLUDE_TRYBOTS=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
==========

to

==========
gpu: GpuFence prototype.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[reveman, 2016-09-30 12:03:34]

Description was changed from

==========
gpu: GpuFence prototype.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
gpu: GpuFence prototype.

NOT FOR REVIEW (yet)

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[reveman, 2016-10-23 18:14:22]

Description was changed from

==========
gpu: GpuFence prototype.

NOT FOR REVIEW (yet)

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
gpu: Add GpuFence framework.

This introduces a new type of synchronization primitive that can be
used to more effectively synchronize access to resources shared with
the GPU. It's useful when the resources dealing with resources that
are used in multiple processes or accessed both using the GPU and the
CPU.
    
It provides the building blocks needed to effectively support
explicit synchronization and reduce the synchronization overhead
for GpuMemoryBuffers on platforms without native buffer support.
    
A software implementation of the GpuFence interface is provided as
part of this patch. This software implementation can be used as
a fallback on platforms without native sync primitives and is
ideal for synchronizing access to GpuMemoryBuffers when backed by
shared memory.
    
BUG=653908
TEST=gfx_unittests, gl_tests
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[reveman, 2016-10-23 18:23:06]

The CQ bit was checked by reveman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-23 18:23:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-23 18:28:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-23 18:28:35]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[reveman, 2016-10-23 22:06:06]

The CQ bit was checked by reveman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-23 22:06:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-23 22:28:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-23 22:28:19]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[reveman, 2016-10-24 15:23:50]

The CQ bit was checked by reveman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-24 15:24:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-24 15:57:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-24 15:57:42]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[reveman, 2016-11-02 14:47:58]

reveman@chromium.org changed reviewers:
+ dcheng@chromium.org, piman@chromium.org

[reveman, 2016-11-02 14:47:59]

Hi,

this is ready for initial review now that we have display latency tests that can
measure the improvement it provides for Arc++. This is just the first in a
serious of patches that provide more efficient CPU access to GMBs in Chrome and
explicit synchronization support for Arc++.

Please take a look at

https://docs.google.com/a/google.com/document/d/1n848e-a_NZFyiBG5WenXj9JtwgcN...
and
https://docs.google.com/a/google.com/document/d/1b3w_IBC2409U7G2yfeDuaZ4hGUXd...

for a list of follow up patches and more details about the design.

+piman for general design
+dcheng for *_messages.h and general security review

[reveman, 2017-01-06 08:22:04]

reveman@chromium.org changed reviewers:
+ ericrk@chromium.org, sunnyps@chromium.org

[reveman, 2017-01-06 08:22:06]

+ericrk, +sonnyps

[ericrk, 2017-03-10 03:04:39]

I'm liking this so far :D - Still wrapping my head around how everything fits
together, but I had a few questions to start out.

https://codereview.chromium.org/2383753002/diff/340001/gpu/ipc/client/gpu_cha...
File gpu/ipc/client/gpu_channel_host.cc (right):

https://codereview.chromium.org/2383753002/diff/340001/gpu/ipc/client/gpu_cha...
gpu/ipc/client/gpu_channel_host.cc:290:
gfx::SharedEvent::DuplicateHandle(source_handle.shared_event_handle);
We don't need to call ShareToProcess here? Just duplicate?

https://codereview.chromium.org/2383753002/diff/340001/ui/gfx/shared_event.cc
File ui/gfx/shared_event.cc (right):

https://codereview.chromium.org/2383753002/diff/340001/ui/gfx/shared_event.cc...
ui/gfx/shared_event.cc:25: constexpr size_t kSharedMemorySize = sizeof(int32_t);
I'm curious about the size efficiency here. From my (limited) understanding of
the shared memory system on some of the Chrome platforms, the minimum size
returnable is a page (~4k), and allocating smaller will just allocate the page
and give you a subset. This would make creating a bunch of 4 byte shared memory
allocations pretty inefficient. Is that something we should consider here? Is
there a piece of this I'm misunderstanding?

https://codereview.chromium.org/2383753002/diff/340001/ui/gl/gl_fence_shared_...
File ui/gl/gl_fence_shared_event.cc (right):

https://codereview.chromium.org/2383753002/diff/340001/ui/gl/gl_fence_shared_...
ui/gl/gl_fence_shared_event.cc:12: shared_event_.Close();
So, to make sure I understand, closing this frees up an FD, but does not unmap
the memory (or break the link that mapped memory has to other procs)?

[reveman, 2017-03-13 12:33:06]

https://codereview.chromium.org/2383753002/diff/340001/gpu/ipc/client/gpu_cha...
File gpu/ipc/client/gpu_channel_host.cc (right):

https://codereview.chromium.org/2383753002/diff/340001/gpu/ipc/client/gpu_cha...
gpu/ipc/client/gpu_channel_host.cc:290:
gfx::SharedEvent::DuplicateHandle(source_handle.shared_event_handle);
On 2017/03/10 at 03:04:39, ericrk wrote:
> We don't need to call ShareToProcess here? Just duplicate?

This looks the same as GpuChannelHost::ShareToGpuProcess. We need the same logic
as all other shmem so it this doesn't match then it needs to be updated.

https://codereview.chromium.org/2383753002/diff/340001/ui/gfx/shared_event.cc
File ui/gfx/shared_event.cc (right):

https://codereview.chromium.org/2383753002/diff/340001/ui/gfx/shared_event.cc...
ui/gfx/shared_event.cc:25: constexpr size_t kSharedMemorySize = sizeof(int32_t);
On 2017/03/10 at 03:04:39, ericrk wrote:
> I'm curious about the size efficiency here. From my (limited) understanding of
the shared memory system on some of the Chrome platforms, the minimum size
returnable is a page (~4k), and allocating smaller will just allocate the page
and give you a subset. This would make creating a bunch of 4 byte shared memory
allocations pretty inefficient. Is that something we should consider here? Is
there a piece of this I'm misunderstanding?

Correct. Each fence will end up using one page of memory. That's not ideal but
I'm not sure we'll be creating enough fences on platforms without a native fence
implementation that this will be a problem. We can try to keep multiple fences
in the same chunk of shared memory but I would prefer to keep this simple as a
start and just use one page per fence.

https://codereview.chromium.org/2383753002/diff/340001/ui/gl/gl_fence_shared_...
File ui/gl/gl_fence_shared_event.cc (right):

https://codereview.chromium.org/2383753002/diff/340001/ui/gl/gl_fence_shared_...
ui/gl/gl_fence_shared_event.cc:12: shared_event_.Close();
On 2017/03/10 at 03:04:39, ericrk wrote:
> So, to make sure I understand, closing this frees up an FD, but does not unmap
the memory (or break the link that mapped memory has to other procs)?

Correct. We do the same for shared bitmaps and discardable memory when possible
for the same reason.

[gurchetansingh, 2017-04-14 21:40:47]

Description was changed from

==========
gpu: Add GpuFence framework.

This introduces a new type of synchronization primitive that can be
used to more effectively synchronize access to resources shared with
the GPU. It's useful when the resources dealing with resources that
are used in multiple processes or accessed both using the GPU and the
CPU.
    
It provides the building blocks needed to effectively support
explicit synchronization and reduce the synchronization overhead
for GpuMemoryBuffers on platforms without native buffer support.
    
A software implementation of the GpuFence interface is provided as
part of this patch. This software implementation can be used as
a fallback on platforms without native sync primitives and is
ideal for synchronizing access to GpuMemoryBuffers when backed by
shared memory.
    
BUG=653908
TEST=gfx_unittests, gl_tests
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
gpu: Add GpuFence framework.

This introduces a new type of synchronization primitive that can be
used to more effectively synchronize access to resources shared with
the GPU. It's useful when the resources dealing with resources that
are used in multiple processes or accessed both using the GPU and the
CPU.
    
It provides the building blocks needed to effectively support
explicit synchronization and reduce the synchronization overhead
for GpuMemoryBuffers on platforms without native buffer support.
    
A software implementation of the GpuFence interface is provided as
part of this patch. This software implementation can be used as
a fallback on platforms without native sync primitives and is
ideal for synchronizing access to GpuMemoryBuffers when backed by
shared memory.
    
BUG=653908
TEST=gfx_unittests, gl_tests
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[reveman, 2017-07-17 13:17:30]

reveman@chromium.org changed reviewers:
+ dcastagna@chromium.org, sadrul@chromium.org

[reveman, 2017-07-17 13:17:32]

+sadrul

FYI, I'm going to seriously start moving this and follow up patches (native
fence primitives and explicit synchronization support) forward now that we have
explicit cross-process/device synchronization on some Intel devices on CrOS. I'm
going to have dcastagna@ do the high level review of each patch and include the
others of you for more specific owner reviews of specific components.