Bug 223713 – Preload graphics drivers on a background thread instead of the main thread

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

Preload graphics drivers on a background thread instead of the main thread. We have evidence of prewarmGlobally() hanging the main thread so we should do pre-warming off the main thread whenever possible.
r265418 introduced this graphics loader preloading and an earlier version of this patch was simply calling MTLCopyAllDevices() on a background queue. However, that patch was updated before landing to do the work on the main thread.
I think we should go back to the earlier iteration.

[Radar WebKit Bug Importer]

<rdar://problem/75804953>

[Chris Dumez]

Created attachment 424186 [details]
Patch

[Geoffrey Garen]

Seems fine to do it this way -- but do we know why MTLCopyAllDevices() hangs?

[Chris Dumez]

(In reply to Geoffrey Garen from comment #3)
> Seems fine to do it this way -- but do we know why MTLCopyAllDevices() hangs?

If you look at the radar, there is no evidence of any hang inside MTLCopyAllDevices() per say. It just looks like prewarmGlobally() can be slow under some circumstances (often in a different place inside prewarmGlobally, not necessarily Metal) and we get a report of main thread hang.

I have zero evidence of an issue with Metal per say but I am looking for work inside prewarmGlobally() that can be moved off the main thread. Metal happens for be something we can move off-thread. I plan to follow-up with other things I am able to move off the main thread.

[EWS]

Committed r274982: <https://commits.webkit.org/r274982>

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

[Kimmo Kinnunen]

Comment on attachment 424186 [details]
Patch

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

> Source/WebCore/platform/graphics/gpu/cocoa/GPUDeviceMetal.mm:49
> +        MTLCopyAllDevices();

(Sorry for not knowing obj-c or the current sdks better)

Does this leak? Below we capture same call result with adoptNS
Does this compile on non-mac, non-catalyst? Below we ifdef it. My SDK fails to compile this, too.

[Ben Nham]

(In reply to Kimmo Kinnunen from comment #6)
> Comment on attachment 424186 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=424186&action=review
> 
> > Source/WebCore/platform/graphics/gpu/cocoa/GPUDeviceMetal.mm:49
> > +        MTLCopyAllDevices();
> 
> (Sorry for not knowing obj-c or the current sdks better)
> 
> Does this leak? Below we capture same call result with adoptNS

The returned array contains references to MTLDevice objects that generally live for the life of the process, but that is an implementation detail that we probably shouldn't count on. (Also I don't think that is true for a GPU that is removed.) So we should probably add a release or an adoptNS here.

> Does this compile on non-mac, non-catalyst? Below we ifdef it. My SDK fails
> to compile this, too.

We should add the PLATFORM(MAC) here because MTLCopyAllDevices is not a public API on iOS. The issue of GPU drivers falling out of the dyld shared cache is also doesn't occur on iOS so we wouldn't want to call it as an SPI during prewarming anyway.

Here is a patch to address these issues: https://bugs.webkit.org/show_bug.cgi?id=223747