1391159 - Shader creation of WebRender took long time

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P1)

Description

[Sotaro Ikeda [:sotaro]]

When WebRender is enabled, some shader creations happened on 2nd rendering. Each shader creation took 50ms - 150ms, then 2nd webrender rendering took about 600ms - 1000ms. The duration of 2nd composition was depended on PC.

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Add log to measure shader creation time

I used the patch to quickly measure shaders creation time.

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: log - mearured shader creation time on Lenovo P50

Measured shaders creation time with attachment 8898127 [details] [diff] [review]. It was measured on Lenovo P50(windows).

Each shader creation took 43 ms - 151 ms and 2nd webrender rendering took 841ms.

Comment 3

[Sotaro Ikeda [:sotaro]]

The following is an advice by Jeff Gilbert via E-mail.

-------------------------------------------------------------------------

My instinct here is that WR should keep a global shader cache itself.
We do want a shader cache system for WebGL as well, so if we can kill
two birds with one stone here, that would be a plus.

I don't think we should try to embed shader caching in GLContext transparently.

Straw-man:

class ShaderProgramCache
{
  map<Hasher::HashT, RefPtr<ProgramBinary>> cache;

  struct ProgramBinary : public RefCounted<ProgramBinary> {
    GLenum format;
    UniqueBuffer bytes;
    uint32_t byteLen;
  };

  void Link(GLContext* gl, GLuint progId, const char* vertSource,
const char* fragSource) {
    Hasher hasher;
    hasher.digest(gl->mRendererString);
    hasher.digest(vertSource);
    hasher.digest(fragSource);
    const auto hash = hasher.get();
    const auto itr = cache.find(hash);
    if (itr != cache.end()) {
      const auto& bin = itr->second;
      gl->fProgramBinary(progId, bin->format, bin->bytes, bin->byteLen);
      return;
    }
    gl->fLinkProgram(progId);
    GLenum status = 0;
    gl->fGetProgramiv(progId, LOCAL_GL_LINK_STATUS, (GLint*)&status);
    if (!status)
      return;
    RefPtr<ProgramBinary> bin = new ProgramBinary;
    gl->fGetProgramiv(progId, LOCAL_GL_PROGRAM_BINARY_LENGTH,
(GLint*)&bin->byteLen);
    bin->bytes = malloc(bin->byteLen);
    if (!bin->bytes)
      return;
    gl->fGetProgramBinary(progId, bin->byteLen, nullptr, &bin->format,
bin->bytes.get());
   cache.insert({hash, bin});
  }
};

Optional requirements:
- Async linking
- Serialize to/from blob/disk

Comment 4

[Sotaro Ikeda [:sotaro]]

I am going to take a look.

Comment 5

[Sotaro Ikeda [:sotaro]]

https://github.com/servo/webrender/pull/1588

Comment 6

[Sotaro Ikeda [:sotaro]]

Attachment: temporal patch - Add log to measure shader creation time

Rebased.

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: wip

Comment 8

[Sotaro Ikeda [:sotaro]]

Without glProgramBinary usage, each shader compiling took 60ms - 500ms with ANGLE on P90(Win10).

With glProgramBinary usage, each shader load became 4ms - 10ms with ANGLE on P90(Win10).

Comment 9

[Sotaro Ikeda [:sotaro]]

Attachment: wip

Comment 10

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> The following is an advice by Jeff Gilbert via E-mail.
> 
> -------------------------------------------------------------------------
> 
> I don't think we should try to embed shader caching in GLContext
> transparently.
> 
> Optional requirements:
> - Async linking
> - Serialize to/from blob/disk

:jgilbert, can you explain more about "Async linking"? Thanks!

Comment 11

[Sotaro Ikeda [:sotaro]]

Attachment: wip

Comment 12

[Sotaro Ikeda [:sotaro]]

Attachment: wip

Comment 13

[Sotaro Ikeda [:sotaro]]

Attachment: wip

Comment 14

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> 
> Optional requirements:
> - Async linking
> - Serialize to/from blob/disk

Optional requirements is going to be handled in different bugs.

Comment 15

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #14)
> (In reply to Sotaro Ikeda [:sotaro] from comment #3)
> > 
> > - Serialize to/from blob/disk

Bug 1418202 is created for it.

Comment 16

[Sotaro Ikeda [:sotaro]]

Attachment: wip

Comment 17

[Sotaro Ikeda [:sotaro]]

Attachment: patch - handle WebRender ProgramBinary usage

Comment 18

[Sotaro Ikeda [:sotaro]]

Attachment: patch - handle WebRender ProgramBinary usage

Comment 19

[Sotaro Ikeda [:sotaro]]

For now, ProgramBinary is necessary for windows. On my linux PC, I did not see performance improvement.

Comment 20

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8929359 [details] [diff] [review]
patch - handle WebRender ProgramBinary usage

:nical, can you feedback to the patch?

Comment 21

[Nicolas Silva [:nical]]

Comment on attachment 8929359 [details] [diff] [review]
patch - handle WebRender ProgramBinary usage

Review of attachment 8929359 [details] [diff] [review]:
-----------------------------------------------------------------

Looks sane.

Comment 22

[Sotaro Ikeda [:sotaro]]

Attachment: wip

Comment 23

[Sotaro Ikeda [:sotaro]]

Attachment: wip

Comment 24

[Sotaro Ikeda [:sotaro]]

Attachment: patch - handle WebRender ProgramBinary usage

Comment 25

[Sotaro Ikeda [:sotaro]]

Attachment: patch - handle WebRender ProgramBinary usage

Comment 26

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Handle WebRender ProgramBinary usage

Comment 27

[Sotaro Ikeda [:sotaro]]

Attachment: temporal patch - Add log to measure shader creation time

Comment 28

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e9716336d12a7da134fb4a92da5a838d3b20ff0

Comment 29

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef497377959b0956f7d40a70237de6e2b57648bc&selectedJob=147335907

Comment 30

[Nicolas Silva [:nical]]

Comment on attachment 8931570 [details] [diff] [review]
patch - Handle WebRender ProgramBinary usage

Review of attachment 8931570 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e60ad275b73
Handle WebRender ProgramBinary usage r=nical

Comment 32

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/7e60ad275b73

Comment 33

[Kelsey Gilbert [:jgilbert]]

(In reply to Sotaro Ikeda [:sotaro] from comment #10)
> (In reply to Sotaro Ikeda [:sotaro] from comment #3)
> > The following is an advice by Jeff Gilbert via E-mail.
> > 
> > -------------------------------------------------------------------------
> > 
> > I don't think we should try to embed shader caching in GLContext
> > transparently.
> > 
> > Optional requirements:
> > - Async linking
> > - Serialize to/from blob/disk
> 
> :jgilbert, can you explain more about "Async linking"? Thanks!

Async linking is where we call LinkProgram for all programs before querying the LINK_STATUS for any of them. This lets the driver do linking in parallel. I believe ANGLE supports this, or something similar.