Bug 237864 - [WebGPU] Create a path of Ref<>s between Instance and Queue
Summary: [WebGPU] Create a path of Ref<>s between Instance and Queue
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on: 237861
Blocks: 237583 237869
  Show dependency treegraph
 
Reported: 2022-03-14 18:52 PDT by Myles C. Maxfield
Modified: 2022-03-16 14:07 PDT (History)
4 users (show)

See Also:


Attachments
Patch (10.33 KB, patch)
2022-03-14 19:00 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (10.12 KB, patch)
2022-03-14 19:05 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (10.21 KB, patch)
2022-03-15 14:47 PDT, Myles C. Maxfield
kkinnunen: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2022-03-14 18:52:44 PDT
Instance has a method to run asynchronous work, and Queue needs to call it.
Comment 1 Myles C. Maxfield 2022-03-14 19:00:22 PDT
Created attachment 454646 [details]
Patch
Comment 2 Myles C. Maxfield 2022-03-14 19:05:31 PDT
Created attachment 454647 [details]
Patch
Comment 3 Kimmo Kinnunen 2022-03-15 06:10:15 PDT
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?
Comment 4 Myles C. Maxfield 2022-03-15 14:37:05 PDT
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.
Comment 5 Myles C. Maxfield 2022-03-15 14:47:24 PDT
Created attachment 454758 [details]
Patch
Comment 6 Kimmo Kinnunen 2022-03-16 05:48:39 PDT
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
Comment 7 Myles C. Maxfield 2022-03-16 14:05:15 PDT
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
Comment 8 Myles C. Maxfield 2022-03-16 14:06:39 PDT
Committed r291365 (248497@trunk): <https://commits.webkit.org/248497@trunk>
Comment 9 Radar WebKit Bug Importer 2022-03-16 14:07:17 PDT
<rdar://problem/90391171>