Bug 220476 – [GPUProcess] Move DOM / Canvas rendering off the main thread in the GPUProcess

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Move DOM / Canvas rendering off the main thread in the GPUProcess.

I am still waiting on A/B perf testing from the bots, but local testing seems to show this is likely perf-neutral:

===
* Rendering off the main thread:
146.87 +/- 9%
149.21 +/- 4%

* Rendering on main thread:
148.29 +/- 2%
145.89 +/- 8%
===

The good news though is that this architecture would allow us to block the thread on a semaphore more aggressively when waiting for new DisplayList entries. We currently are not able to be as aggressive as we'd like because we'd block the main thread.

[Chris Dumez]

Created attachment 417293 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 417293 [details]
Patch

Does this move all ImageBuffer IPC receiving onto a single thread that's not the main thread?

[Chris Dumez]

(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 417293 [details]
> Patch
> 
> Does this move all ImageBuffer IPC receiving onto a single thread that's not
> the main thread?

It moves all *RemoteRenderingBackend* IPC receiving to a *serial workqueue* that's not the main thread. This means all the following IPC messages:
    CreateImageBuffer(WebCore::FloatSize logicalSize, WebCore::RenderingMode renderingMode, float resolutionScale, WebCore::ColorSpace colorSpace, enum:uint8_t WebCore::PixelFormat pixelFormat, WebCore::RenderingResourceIdentifier renderingResourceIdentifier)
    WakeUpAndApplyDisplayList(struct WebKit::GPUProcessWakeupMessageArguments arguments)
    GetImageData(enum:uint8_t WebCore::AlphaPremultiplication outputFormat, WebCore::IntRect srcRect, WebCore::RenderingResourceIdentifier renderingResourceIdentifier) -> (IPC::ImageDataReference imageData) Synchronous
    GetDataURLForImageBuffer(String mimeType, Optional<double> quality, enum:uint8_t WebCore::PreserveResolution preserveResolution, WebCore::RenderingResourceIdentifier renderingResourceIdentifier) -> (String urlString) Synchronous
    GetDataForImageBuffer(String mimeType, Optional<double> quality, WebCore::RenderingResourceIdentifier renderingResourceIdentifier) -> (Vector<uint8_t> data) Synchronous
    GetBGRADataForImageBuffer(WebCore::RenderingResourceIdentifier renderingResourceIdentifier) -> (Vector<uint8_t> data) Synchronous
    CacheNativeImage(WebKit::ShareableBitmap::Handle handle, WebCore::RenderingResourceIdentifier renderingResourceIdentifier)
    CacheFont(IPC::FontReference font)
    DeleteAllFonts()
    DidCreateSharedDisplayListHandle(WebCore::DisplayList::ItemBufferIdentifier identifier, WebKit::SharedMemory::IPCHandle handle, WebCore::RenderingResourceIdentifier destinationBufferIdentifier)
    ReleaseRemoteResource(WebCore::RenderingResourceIdentifier renderingResourceIdentifier)


I am not sure what you mean my ImageBuffer IPC. I don't know see a messages.in file corresponding to ImageBuffer specifically.

[Tim Horton]

RemoteRenderingBackend is the IPC proxy through which ImageBuffer work flows

[Chris Dumez]

Created attachment 417320 [details]
Patch

[Chris Dumez]

A/B testing came back. The current iteration of the patch seems to be a close to 1% regression on MotionMark unfortunately.

[Chris Dumez]

Created attachment 417422 [details]
Patch

[Chris Dumez]

Created attachment 417500 [details]
Patch

[Chris Dumez]

Created attachment 417508 [details]
Patch

[Chris Dumez]

Created attachment 417567 [details]
Patch

[Chris Dumez]

Created attachment 417568 [details]
Patch

[Chris Dumez]

Created attachment 417581 [details]
Patch

[Chris Dumez]

Ping review?

[Radar WebKit Bug Importer]

<rdar://problem/73260041>

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 417581 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Chris Dumez]

Created attachment 417726 [details]
Patch

[EWS]

Committed r271533: <https://trac.webkit.org/changeset/271533>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417726 [details].

[youenn fablet]

With this patch, RemoteRenderingBackend::cacheNativeImage is called in a background thread. This can call ShareableBitmap::createGraphicsContext which has an assertion to be called in main thread.
This reproduces for me by running webrtc/video.html with GPU process enabled in WTR/debug.

[youenn fablet]

(In reply to youenn fablet from comment #18)
> With this patch, RemoteRenderingBackend::cacheNativeImage is called in a
> background thread. This can call ShareableBitmap::createGraphicsContext
> which has an assertion to be called in main thread.
> This reproduces for me by running webrtc/video.html with GPU process enabled
> in WTR/debug.

https://build.webkit.org/results/Apple-Catalina-Debug-WK2-GPUProcess-Tests/r271596%20(5773)/results.html

[Chris Dumez]

(In reply to youenn fablet from comment #18)
> With this patch, RemoteRenderingBackend::cacheNativeImage is called in a
> background thread. This can call ShareableBitmap::createGraphicsContext
> which has an assertion to be called in main thread.
> This reproduces for me by running webrtc/video.html with GPU process enabled
> in WTR/debug.

Ok, will look now. Thanks.

[Chris Dumez]

(In reply to Chris Dumez from comment #20)
> (In reply to youenn fablet from comment #18)
> > With this patch, RemoteRenderingBackend::cacheNativeImage is called in a
> > background thread. This can call ShareableBitmap::createGraphicsContext
> > which has an assertion to be called in main thread.
> > This reproduces for me by running webrtc/video.html with GPU process enabled
> > in WTR/debug.
> 
> Ok, will look now. Thanks.

Follow-up in <https://trac.webkit.org/changeset/271601>.