1347005 - Reuse ANGLE shader compilers

Status: NEW

(Core :: Graphics: CanvasWebGL, enhancement, P3)

Description

[Kelsey Gilbert [:jgilbert]]

We're supposed to be reusing them, and doing so should help both memory usage and shader compilation performance.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1347005 - Reuse ANGLE compilers and simplify shader validation -

Review commit: https://reviewboard.mozilla.org/r/121096/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/121096/

Comment 2

[Michael Leu[:Lenzak](UTC+8)]

This patch looks good to me overall.

However, the shader building time is slower by about 0.1sec slower on EpicZenGarden demo after this patch applied, is that normal?

Here is the shader build time I ran on Firefox Nightly, 3 times for each.

w/o patch:
8.03sec
8.02sec
8.03sec

w/ patch
8.15sec
8.18sec
8.18sec

Comment 3

[Kelsey Gilbert [:jgilbert]]

(In reply to Michael Leu[:Lenzak](UTC+8) from comment #2)
> This patch looks good to me overall.
> 
> However, the shader building time is slower by about 0.1sec slower on
> EpicZenGarden demo after this patch applied, is that normal?
> 
> Here is the shader build time I ran on Firefox Nightly, 3 times for each.
> 
> w/o patch:
> 8.03sec
> 8.02sec
> 8.03sec
> 
> w/ patch
> 8.15sec
> 8.18sec
> 8.18sec

Hmm, ~2% regression based on these numbers. Still, let's take this for simplification, at least.

Comment 4

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8848122 [details]
Bug 1347005 - Reuse ANGLE compilers and simplify shader validation -

https://reviewboard.mozilla.org/r/121096/#review123798

::: dom/canvas/WebGLProgram.h:160
(Diff revision 1)
>      bool UseProgram() const;
>      void ValidateProgram() const;
>  
>      ////////////////
>  
> -    bool FindAttribUserNameByMappedName(const nsACString& mappedName,
> +    RefPtr<const webgl::LinkedProgramInfo> GatherLinkInfo() const;

Maybe using already_AddRefed<> as return type is better?

Comment 5

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8848122 [details]
Bug 1347005 - Reuse ANGLE compilers and simplify shader validation -

https://reviewboard.mozilla.org/r/121096/#review124264

I think it is slightly slower because of the intensive string compare jobs, but the code does look simpler.

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8848122 [details]
Bug 1347005 - Reuse ANGLE compilers and simplify shader validation -

https://reviewboard.mozilla.org/r/121096/#review123798

> Maybe using already_AddRefed<> as return type is better?

already_AddRefed is deprecated now that RefPtr has a move constructor. I'll file a bug to make this more clear.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information