Closed
Bug 1492723
Opened 6 years ago
Closed 6 years ago
Reduce cost of shader compilation in WebRender startup
Categories
(Core :: Graphics: WebRender, enhancement, P1)
Core
Graphics: WebRender
Tracking
()
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: u480271, Assigned: jgilbert)
References
Details
Attachments
(7 files)
21.75 KB,
patch
|
Details | Diff | Splinter Review | |
5.51 KB,
patch
|
Details | Diff | Splinter Review | |
15.70 KB,
patch
|
Details | Diff | Splinter Review | |
5.11 KB,
patch
|
Details | Diff | Splinter Review | |
1.81 KB,
patch
|
Details | Diff | Splinter Review | |
30.78 KB,
patch
|
Details | Diff | Splinter Review | |
23.62 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Was seeing the creation of debug shaders on creation of a new window with Ctrl+N in gecko. MozReview-Commit-ID: 9p3D2hLSL8r
(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #1) > Created attachment 9010533 [details] [diff] [review] > Hack ANGLE to add EGL_MOZ_create_context_provoking_vertex_dont_care > extension. > > MozReview-Commit-ID: ExeqEnRtmpB This patch adds the ability to disable ANGLE's generation of geometry shaders to match the OpenGL specified behavior for "flat" attributes between VS-FS. WebRender's shaders don't depend on the provoking vertex order. This is reported to ANGLE in issue 2829 (https://bugs.chromium.org/p/angleproject/issues/detail?id=2829)
(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #2) > Created attachment 9010534 [details] [diff] [review] > Use EGL_MOZ_context_create_provoking_vertex_dont_care > > MozReview-Commit-ID: It8fOzqiFIe Use EGL_MOZ_create_context_provoking_vertex_dont_care extension when creating context for WebRender.
Reporter | ||
Comment 10•6 years ago
|
||
(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #3) > Created attachment 9010535 [details] [diff] [review] > One, shared GLContext in RenderThread > > MozReview-Commit-ID: 4RHhlk53R5f Create one GL context for all WebRender to use, owned by the RenderThread. This is to facilitate the sharing of shaders across all WebRender windows.
Reporter | ||
Comment 11•6 years ago
|
||
(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #10) > Create one GL context for all WebRender to use, owned by the RenderThread. > This is to facilitate the sharing of shaders across all WebRender windows. This change only creates an ANGLE context. There is more work required to implement CreateGLContextOGL() for non-ANGLE contexts.
Reporter | ||
Comment 12•6 years ago
|
||
(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #4) > Created attachment 9010536 [details] [diff] [review] > Remove unused _file_changed_handler > > MozReview-Commit-ID: LIH83gKxaw2 _file_changed_handler isn't used. Removing it caused an avalanche of dead code.
Reporter | ||
Comment 13•6 years ago
|
||
(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #5) > Created attachment 9010537 [details] [diff] [review] > Don't create a debug_renderer if there is nothing to render. > > Was seeing the creation of debug shaders on creation of a new window with > Ctrl+N in gecko. > > MozReview-Commit-ID: 9p3D2hLSL8r Debug shaders are created even when there is no debug rendering. https://github.com/servo/webrender/issues/3089
Reporter | ||
Comment 14•6 years ago
|
||
(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #6) > Created attachment 9010538 [details] [diff] [review] > Shader pre-caching and sharing amongst windows. > > MozReview-Commit-ID: 3EHw9hdfKwe Create a Shaders struct early in RenderThread startup and hand it to each created WebRender Renderer created in wr_window_new.
Reporter | ||
Comment 15•6 years ago
|
||
(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #7) > Created attachment 9010539 [details] [diff] [review] > Add stratified shader precaching > > MozReview-Commit-ID: GEfwKbLFetq Split the pre-caching concept into two tiers. The "essential" shaders required to get to a basic page like about:newtab and "all" shaders.
Reporter | ||
Comment 16•6 years ago
|
||
https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FeX26uatBTkuAnbX6Ft7_0Q%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_sessionrestore.zip/calltree/?file=profile_sessionrestore%2Fstartup%2Fcycle_3.profile&globalTrackOrder=1-0-2-3-4-5&hiddenGlobalTracks=1&localTrackOrderByPid=6528-1-0~2660-0-1-2-3-4-5-6-7-8-9-10-11-12~&range=0.0000_2.2274~0.0000_1.0608~0.0000_0.4305&thread=7&v=3 Outstanding issue is that shader compilation takes 190ms on RenderThread. A number of ideas have been canvased to improve the situation, mostly focusing on ideas to parallelize the compilation: a) Use GL_KHR_parallel_shader_compile extension b) Create a thread inside wr::Renderer::new() to dispatch shader compilation to c) Create a thread inside RenderThread to dispatch shader compilation to Each one has issues. a) This feature is implemented in ANGLE and gives ANGLE a thread pool to compile shaders upon. The API requires refactoring the way shaders are loaded to not cause a stall because as soon as an OpenGL function is issues against the program. The only non-blocking call is glGetProgramiv(GL_COMPLETION_STATUS_KHR) which will inform the called if program linking has completed. To use this in WebRender, Device::create_program will need to be refactored to check the link status on render to delay blocking on waiting for the link to complete. b) :gw suggested the following: > ===== current ===== > > renderer::new() > let device = ... > create some GL resources with device > let shaders = Shaders::new(&device) > create some more GL resources > > > ===== proposed ===== > > renderer::new() > let device = ... > create (cheap) non-shader GL resources > let (tx, rx) = channel(); > // spawn thread which takes ownership of device > spawn(|| { > let shaders = Shaders::new(&device); > tx.send(shaders, device); > }) > > renderer::draw() > // if draw is invoked for the first time without shaders, block until thread returns them > if self.shaders.is_none() { > (self.shaders, self.device) = self.rx.recv().unwrap() > } The idea was to move Device and Shaders to another thread so that they couldn't be accidentally used. Unfortunately Device can't be moved because it holds Rc<>. :gw suggests removing the need to pass the Device to another thread but that requires calling directly to the GL context to do the creation. c) We are blocking because wr_window_new requires the Device and Shaders at be passed into the creation of wr::Renderer. We could spin up a thread pool and create a GL context per thread that shares the main context and just causes the program to be linked.
Updated•6 years ago
|
Blocks: stage-wr-trains
Priority: P3 → P2
Assignee | ||
Comment 17•6 years ago
|
||
I'll at least get the ANGLE requirements satisfied: https://bugs.chromium.org/p/angleproject/issues/detail?id=2829
Assignee: nobody → jgilbert
Updated•6 years ago
|
Priority: P2 → P1
Comment 18•6 years ago
|
||
(Tentatively marking this as P1 because it's pretty important, but we could have a conversation about riding to beta without it).
Assignee | ||
Comment 19•6 years ago
|
||
So Jamie Madill (Google) has this recommendation, in context of concurrent glProgramBinary:
> JG: What are our other startup options?
> JM: hey, depends on what you're doing, but you can use the ANGLE internal program cache control extension or the blob cache extension
> i think we're transitioning to the blob cache extension
> is this binary from an internal cache in webrender?
> JG: Yeah, we load it out of a disk cache
> JM: cool, that's precisely what the blob cache extension can do
> and the cache control extension too if that one doesn't meet your needs
> https://www.khronos.org/registry/EGL/extensions/ANDROID/EGL_ANDROID_blob_cache.txt
> JG: We'll take a look!
> JM: https://chromium.googlesource.com/angle/angle/+/master/extensions/ANGLE_program_cache_control.txt
> JG: cool
> JM: oh and this https://chromium.googlesource.com/angle/angle/+/master/extensions/EGL_ANGLE_program_cache_control.txt
> JG: Oh weird, I get it. This covers shader recompilation?
> JM: yeah i think they both get triggers when there is a dynamic shader recompile
> during a draw call
Comment 20•6 years ago
|
||
I took this patch queue, added a prototype for bug 1494474 (async glProgramBinary) plus some WebRender changes to avoid accidentally flushing too early, and it looks like that's going to be sufficient to fix the startup regressions: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0333b04b623a0b6f2835c67d7c9aabed79b17f7 I'll start working on cleaning the patches up so that we can land them asap.
Comment 21•6 years ago
|
||
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/cb58c73f2d5f Use EGL_MOZ_context_create_provoking_vertex_dont_care. r=mattwoodrow
Comment 22•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cb58c73f2d5f
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in
before you can comment on or make changes to this bug.
Description
•