1060085 - Add Thread-local versions of ShSurf::Fence/Poll/Wait

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Kelsey Gilbert [:jgilbert]]

We get access to some better synchronization opportunities if we have thread-local versions of ShSurf::Poll and Wait.

For instance, if we add a frame of delay, but call poll/wait on the content thread, we can wait until then to readback, which may allow for better throughput than doing readback during Fence().

Also, NV_fence for ANGLE has to happen on the same GLContext, so we can only use fTestFence and fFinishFence on the same thread we make the fence on.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: patch 1: Add entrypoints and add NV_fence support to ANGLE ShSurfs

Comment 2

[Kelsey Gilbert [:jgilbert]]

Attachment: patch 2: Add thread-local Fence/Poll/Wait to Basic ShSurf

Comment 3

[Kelsey Gilbert [:jgilbert]]

  https://tbpl.mozilla.org/?tree=Try&rev=8ce046a47fdc

Comment 4

[u480271]

Comment on attachment 8480887 [details] [diff] [review]
patch 1: Add entrypoints and add NV_fence support to ANGLE ShSurfs

Review of attachment 8480887 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/SharedSurface.h
@@ +86,5 @@
>  
> +    // Use these if you can. They can only be called from the Content
> +    // thread, though!
> +    virtual void Fence_ThreadLocal() {
> +        Fence();

Should we assert the thread it's being called from? I'm not a huge fan of _ThreadLocal(). Maybe _ContentThread, or something. I don't know, but I also don't think it warrants an r-.

Comment 5

[u480271]

Comment on attachment 8480891 [details] [diff] [review]
patch 2: Add thread-local Fence/Poll/Wait to Basic ShSurf

Review of attachment 8480891 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/SharedSurfaceGL.cpp
@@ +72,5 @@
>      , mTex(tex)
>      , mFB(0)
> +    , mIsDataCurrent(false)
> +#ifdef DEBUG
> +    , mParentThread(NS_GetCurrentThread())

Should this be mOwningThread, perhaps?  Parent implies hierarchy to me (object-to-object). In this case, I think a thread "owns" the fence/surf/etc.

Comment 6

[Kelsey Gilbert [:jgilbert]]

Attachment: patch 1: Add entrypoints and add NV_fence support to ANGLE ShSurfs

I changed the name and made it assert without having to add asserts to each implementation.

Comment 7

[Kelsey Gilbert [:jgilbert]]

Attachment: patch 2: Add thread-local Fence/Poll/Wait to Basic ShSurf

This is just a de-bitrotted version of the previous r+.
r=kamidphish

Comment 8

[Kelsey Gilbert [:jgilbert]]

  https://hg.mozilla.org/integration/mozilla-inbound/rev/d1bc9a5c17ff
  https://hg.mozilla.org/integration/mozilla-inbound/rev/9d4fc9f2d5d1

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d1bc9a5c17ff
https://hg.mozilla.org/mozilla-central/rev/9d4fc9f2d5d1