Bug 224991 - [Metal ANGLE] Temporarily remove AST validation even for debug builds
Summary: [Metal ANGLE] Temporarily remove AST validation even for debug builds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-23 13:25 PDT by John Cunningham
Modified: 2021-04-25 12:16 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.38 KB, patch)
2021-04-23 13:26 PDT, John Cunningham
no flags Details | Formatted Diff | Diff
Patch (1.55 KB, patch)
2021-04-23 14:15 PDT, John Cunningham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Cunningham 2021-04-23 13:25:24 PDT
[Metal ANGLE] Temporarily remove AST validation even for debug builds
Comment 1 John Cunningham 2021-04-23 13:26:34 PDT
Created attachment 426937 [details]
Patch
Comment 2 John Cunningham 2021-04-23 13:26:37 PDT
<rdar://problem/76299178>
Comment 3 EWS Watchlist 2021-04-23 13:27:22 PDT
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment 4 Kenneth Russell 2021-04-23 13:59:43 PDT
Could you please provide a description in this bug of why this is being done, and how temporarily this is being removed?
Comment 5 John Cunningham 2021-04-23 14:08:36 PDT
Sure thing. We are seeing significant slow downs inside of the validateAST calls on debug builds leading to test timeouts. 

There are a significant number of transform passes as a part of the GLSL AST -> MSL translation, and currently each pass is validating the AST. Theses were mainly in place during development of the translator, and the correct fix is to move the validation to the very end after all of the transformations happen. To unblock further testing, we'd like to land this stopgap to run all of the debug testing, until we land the more correct fix (hopefully within the next few days.)
Comment 6 Kenneth Russell 2021-04-23 14:09:44 PDT
Comment on attachment 426937 [details]
Patch

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

Thanks for the explanation. r+

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ShaderMtl.mm:88
> +#if defined(ANGLE_ENABLE_ASSERTS) && 0

Please add some TODO here regarding moving this validation in the future so we don't forget it.
Comment 7 John Cunningham 2021-04-23 14:15:32 PDT
Created attachment 426942 [details]
Patch
Comment 8 Kenneth Russell 2021-04-23 14:16:31 PDT
Comment on attachment 426942 [details]
Patch

Thanks, still looks good. r+ again
Comment 9 EWS 2021-04-25 12:16:33 PDT
Committed r276568 (237004@main): <https://commits.webkit.org/237004@main>

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