Bug 215183

Summary: Preload graphics drivers in Mac WebProcess
Product: WebKit Reporter: Ben Nham <nham>
Component: New BugsAssignee: Ben Nham <nham>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, darin, dino, ews-watchlist, ggaren, jonlee, nham, sabouhallawa, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=223713
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Ben Nham 2020-08-05 12:17:04 PDT 
Asynchronously preload graphics drivers in Mac WebProcess
Comment 1 Ben Nham 2020-08-05 12:19:32 PDT 
Created attachment 406023 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2020-08-05 12:20:19 PDT 
<rdar://problem/66587606>
Comment 3 Ben Nham 2020-08-05 12:35:52 PDT 
Created attachment 406025 [details]
Patch
Comment 4 Simon Fraser (smfr) 2020-08-05 13:54:47 PDT 
Comment on attachment 406025 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406025&action=review

> Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:227
> +    if (settings.acceleratedDrawingEnabled()) {

I would not bother checking this. It's the default, and even if it isn't, canvas or WebGL can use Metal.
Comment 5 Tim Horton 2020-08-05 17:46:14 PDT 
Also, Darin will show up shortly to tell you that you don’t need the dispatch_once since this code is already not threadsafe; just use a static bool.
Comment 6 Ben Nham 2020-08-05 17:48:43 PDT 
If I don't need to check the setting, then I'll probably move this to earlier in the init process (e.g. to WebProcess::platformInitializeWebProcess or WebProcess::platformSetWebsiteDataStoreParameters).
Comment 7 Tim Horton 2020-08-05 17:57:53 PDT 
Definitely don't need to.
Comment 8 Ben Nham 2020-08-06 13:37:31 PDT 
Created attachment 406110 [details]
Patch
Comment 9 Alexey Proskuryakov 2020-08-07 10:45:27 PDT 
Comment on attachment 406110 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406110&action=review

> Source/WebCore/page/ProcessWarming.cpp:47
> +#include <Metal/Metal.h>

Looks like this is an Objective-C header.
Comment 10 Alexey Proskuryakov 2020-08-07 10:45:53 PDT 
(so the patch breaks the build)
Comment 11 Geoffrey Garen 2020-08-07 13:05:56 PDT 
Comment on attachment 406110 [details]
Patch

Seems like you could call WebCore::GPUDevice::tryCreate() to make this behavior C++-friendly.
Comment 12 Ben Nham 2020-08-07 17:55:58 PDT 
Created attachment 406236 [details]
Patch
Comment 13 Ben Nham 2020-08-08 14:55:03 PDT 
Comment on attachment 406236 [details]
Patch

I profiled CompPLT with this patch applied and now see all MTLCopyAllDevices work happening in prewarmGlobally as expected.
Comment 14 Darin Adler 2020-08-08 15:02:50 PDT 
Comment on attachment 406236 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406236&action=review

> Source/WebCore/page/ProcessWarming.cpp:89
> +    GPUDevice::tryCreate(gpuRequestAdapterOptions);

Why is the local variable needed? Isn’t this the same as passing WTF::nullopt?
Comment 15 Ben Nham 2020-08-08 16:10:07 PDT 
Created attachment 406261 [details]
Patch
Comment 16 EWS 2020-08-09 11:23:49 PDT 
Committed r265418: <https://trac.webkit.org/changeset/265418>

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