Bug 237864 – [WebGPU] Create a path of Ref<>s between Instance and Queue

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGPU

[Myles C. Maxfield]

Instance has a method to run asynchronous work, and Queue needs to call it.

[Myles C. Maxfield]

Created attachment 454646 [details]
Patch

[Myles C. Maxfield]

Created attachment 454647 [details]
Patch

[Kimmo Kinnunen]

Comment on attachment 454647 [details]
Patch

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

> Source/WebGPU/WebGPU/Device.h:89
> +

Is there a reason this is duplicated across the whole hierarchy?
In the end there's only one item that can schedule work: the Instance.

Why are we holding device that holds adapter that holds instance instead of holding instance directly?

[Myles C. Maxfield]

Comment on attachment 454647 [details]
Patch

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

>> Source/WebGPU/WebGPU/Device.h:89
>> +
> 
> Is there a reason this is duplicated across the whole hierarchy?
> In the end there's only one item that can schedule work: the Instance.
> 
> Why are we holding device that holds adapter that holds instance instead of holding instance directly?

Yeah, I think you're right that this can be more straightforward.

Objects are going to need the device pointer eventually, though, to implement the "valid to use with" check: https://gpuweb.github.io/gpuweb/#abstract-opdef-valid-to-use-with

So I think the chain can be Objects -> Device -> Instance, rather than Objects -> Other objects -> Device -> Adapter -> Instance.

[Myles C. Maxfield]

Created attachment 454758 [details]
Patch

[Kimmo Kinnunen]

Comment on attachment 454758 [details]
Patch

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

> Source/WebGPU/WebGPU/Queue.h:61
> +    void scheduleWork(Instance::WorkItem&&);

Still, I think it's a bit strange to have the schedulework per class.


I'd imagine it'd be:
void Queue::someFunction()
{
    instance()->scheduleWork()
}

Queue::someFunction
  Instance::scheduleWork

instead of:
Queue::someFunction
  Queue::scheduleWork
    Device::scheduleWork
       Instance::scheduleWork

[Myles C. Maxfield]

Comment on attachment 454758 [details]
Patch

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

>> Source/WebGPU/WebGPU/Queue.h:61
>> +    void scheduleWork(Instance::WorkItem&&);
> 
> Still, I think it's a bit strange to have the schedulework per class.
> 
> 
> I'd imagine it'd be:
> void Queue::someFunction()
> {
>     instance()->scheduleWork()
> }
> 
> Queue::someFunction
>   Instance::scheduleWork
> 
> instead of:
> Queue::someFunction
>   Queue::scheduleWork
>     Device::scheduleWork
>        Instance::scheduleWork

Right, makes sense. The Queue::scheduleWork() is valuable, I think, so I think the best option is in the middle:

Queue::someFunction
  Queue::scheduleWork
      Instance::scheduleWork

[Myles C. Maxfield]

Committed r291365 (248497@trunk): <https://commits.webkit.org/248497@trunk>

[Radar WebKit Bug Importer]

<rdar://problem/90391171>