Bug 228179 - Stop building WebGPU and the WHLSL compiler to decrease binary size
Summary: Stop building WebGPU and the WHLSL compiler to decrease binary size
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-22 02:08 PDT by Myles C. Maxfield
Modified: 2021-07-29 20:54 PDT (History)
36 users (show)

See Also:


Attachments
Patch (1.35 KB, patch)
2021-07-22 02:09 PDT, Myles C. Maxfield
mmaxfield: review-
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (2.82 MB, patch)
2021-07-28 02:13 PDT, Myles C. Maxfield
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (2.82 MB, patch)
2021-07-28 02:24 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (3.71 MB, patch)
2021-07-28 18:04 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (3.72 MB, patch)
2021-07-29 02:22 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (3.72 MB, patch)
2021-07-29 04:01 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (3.76 MB, patch)
2021-07-29 13:26 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (3.78 MB, patch)
2021-07-29 13:55 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (3.77 MB, patch)
2021-07-29 16:36 PDT, Myles C. Maxfield
dino: review+
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 2021-07-22 02:08:05 PDT
[Cocoa] Stop building WebGPU and the WHLSL compiler to decrease binary size
Comment 1 Myles C. Maxfield 2021-07-22 02:09:13 PDT
Created attachment 433998 [details]
Patch
Comment 2 Sam Weinig 2021-07-22 10:39:38 PDT
Having code that isn't compiled isn't usually something we like to do. Perhaps we should instead remove it.
Comment 3 Myles C. Maxfield 2021-07-28 01:56:51 PDT
We need a solution for GPU_DRIVER_PREWARMING.
Comment 4 Myles C. Maxfield 2021-07-28 02:13:01 PDT
Created attachment 434414 [details]
Patch
Comment 5 EWS Watchlist 2021-07-28 02:16:24 PDT
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.
Comment 6 Myles C. Maxfield 2021-07-28 02:24:51 PDT
Created attachment 434415 [details]
Patch
Comment 7 Myles C. Maxfield 2021-07-28 18:04:17 PDT
Created attachment 434483 [details]
Patch
Comment 8 Radar WebKit Bug Importer 2021-07-29 02:11:11 PDT
<rdar://problem/81260535>
Comment 9 Myles C. Maxfield 2021-07-29 02:22:07 PDT
Created attachment 434506 [details]
Patch
Comment 10 Myles C. Maxfield 2021-07-29 04:01:02 PDT
Created attachment 434514 [details]
Patch
Comment 11 Myles C. Maxfield 2021-07-29 12:21:02 PDT
Comment on attachment 434514 [details]
Patch

GTK failure looks unrelated
Comment 12 Myles C. Maxfield 2021-07-29 13:26:04 PDT
Created attachment 434565 [details]
Patch
Comment 13 Don Olmstead 2021-07-29 13:32:07 PDT
The Dawn stuff is currently just stubs so get rid of those as well. The EWS won’t catch any build errors since it’s not enabled by default.

Also you’ll want to grep the CMake stuff for anything ENABLE_WEBGPU. I think you can probably ignore the Source/cmake directory in that since eventually this will all be rebooted and that code would need to go back.
Comment 14 Myles C. Maxfield 2021-07-29 13:55:53 PDT
Created attachment 434569 [details]
Patch
Comment 15 Darin Adler 2021-07-29 14:16:42 PDT
I presume the WHLSL is not coming back, but WebGPU? I am a little confused about the status of that.
Comment 16 Myles C. Maxfield 2021-07-29 14:27:18 PDT
Our current implementation of WebGPU:
1. Is off by default on all platforms
2. Is extremely outdated
3. Has no notion of the GPU Process, and therefore needs to be redesigned and largely rewritten
4. Only implements a fraction of what is in the spec

Removing the code from the tree doesn't delete it from existence; it's still in source control. The benefit of reducing binary size seems to outweigh having this code in the tree.
Comment 17 Myles C. Maxfield 2021-07-29 14:34:52 PDT
(also https://bugs.webkit.org/show_bug.cgi?id=228179#c2)
Comment 18 Darin Adler 2021-07-29 15:04:15 PDT
Sure, Myles, I wasn’t questioning the technique.

I was wondering what our WebGPU development strategy was for WebKit. Thanks for filling me in.
Comment 19 Myles C. Maxfield 2021-07-29 15:19:43 PDT
I need to re-revert the changes to Source/WebInspectorUI
Comment 20 Robin Morisset 2021-07-29 15:31:27 PDT
LGTM, but I'd rather let a Webcore developer give this the r+.
Comment 21 Myles C. Maxfield 2021-07-29 15:47:48 PDT
I think our intentions may be a bit muddled here; let me try to make them clear.

1. We desire to support WebGPU in WebKit. If somehow an oracle delivered a high-quality implementation of WebGPU today, we would definitely incorporate it and enable it!
2. We do not desire to support WHLSL in WebKit. WGSL is the path forward.
3. I don't believe there should/will be any development-practices kind of change to the development of WebGPU in WebKit. Development doesn't need to be done on a branch - our usual process of landing patches one by one to trunk continues to make sense.
4. Any implementation of WebGPU in WebKit would have to be GPU-Process-savvy.
5. It is still a good idea for the implementation of WebGPU to be split into two parts: a frontend part that knows things about IDL and promises and javascript, and a backend part that knows things about the backend APIs (like Metal or D3D or Vulkan). The reason is that we may want to implement future features in WebKit (maybe GPU-accelerated SVG filters???) on top of the backend part, without involving Javascript at all. The current implementation puts the frontend in Modules/WebGPU and the backend in platform/graphics/gpu, but those paths aren't set in stone.
6. This patch deletes the existing implementation just to reduce binary size in releases of WebKit which don't enable WebGPU by default. It doesn't represent a change in direction or policy or anything regarding WebGPU. It's a (somewhat temporary) pragmatic change.
Comment 22 Devin Rousso 2021-07-29 16:02:34 PDT
(In reply to Myles C. Maxfield from comment #19)
> I need to re-revert the changes to Source/WebInspectorUI

Yeah, unless your goal is to remove Web Inspector support for WebGPU/WHLSL when inspecting iOS 14.0/14.5/15.0 devices, please undo all your changes inside `Source/WebInspectorUI`.  FWIW it's not unprecedented to also remove Web Inspector backwards compatibility support for features when they're removed from ToT, but we usually prefer not to do that with the goal of keeping Web Inspector as much "as it was" (specifically the functionality, not the UI) as possible.

Also, the JSON files you modified are not actually used anywhere in the frontend.  They're purely for record keeping and archival purposes.  If you did want to do the above, you'd actually want to modify the various `Source/WebInspectorUI/UserInterface/Protocol/Legacy/*/InspectorBackendCommands.js` files as that's actually used by the frontend.  The JSON files are used to generate the JS files, but the JS file doesn't have everything in the JSON file so we keep the JSON file around so we can reference it when needed.

I personally think we should keep supporting Web Inspector backwards compatibility, so I'm in favor of allowing the Web Inspector frontend to still see/instrument/etc. WebGPU/WHLSL when inspecting iOS 14.0/14.5/15.0, but that's just my thoughts.

Also the inspector changes in WebCore look good to me :)
Comment 23 Myles C. Maxfield 2021-07-29 16:36:12 PDT
Created attachment 434588 [details]
Patch
Comment 24 Myles C. Maxfield 2021-07-29 16:57:53 PDT
I added WebGPU to webkit.org/status in https://commits.webkit.org/r280456 with a status of "in progress".
Comment 25 Dean Jackson 2021-07-29 17:06:35 PDT
Comment on attachment 434588 [details]
Patch

Can you mention in the change log that this isn't because we don't want to support WebGPU, but rather that this implementation is incorrect and not serving any purpose (other than slowing down the build). We'll implement a better version.

Also, you say this is for Cocoa, but also remove the Dawn code that was added by other contributors. I guess this makes sense since you're removing the feature.
Comment 26 Myles C. Maxfield 2021-07-29 20:34:51 PDT
I'm probably going to have to land this manually, because the patch is so large
Comment 27 Myles C. Maxfield 2021-07-29 20:54:05 PDT
Committed r280467 (240102@main): <https://commits.webkit.org/240102@main>