Re: [dev-servo] New Servo WebGL Architecture Proposal (benchmarks included!)
Hi Emilio! > Yeah, I'm to blame for this one... This was one of my very first > contributions to Servo and I followed what Canvas2D did (which is > equally broken), and I never got around to clean it up. > Can't wait to see it going away! It's normal to follow a existing pattern in the code in your first contribution. Also things have changed a lot since you started: WR was not ready, you didn't have the external image API and more. So it's easier to make this kind of changes now. In any case I only mentioned the "bad" things. All the WebGL work you did in the DOM/script component, all the validations, error checking, conformance tests is great. The same for the WebGLCommand enum and traits. All that code is still there and WebGL will be imposible without all that work ;) Yes, Canvas2D is broken too. Any volunteers to fix that? ;) > (and hopefully split it into a few commits? That way would make > reviewing it much easier... reviewing the WebVR landing was honestly > quite a pain, and I'm sure some things slipped through because of it was > a single massive commit) I know that the PR was big, sorry about that. Ok, I'll try to reduce the PR as much a as possible. Some of the webgl modes I mentioned are just tests to validate ideas and measure performance differences. Hopefully the review will be a lot easier this time. Github shows a lot of changes in the DOM objects but it's a repeated change (remove CanvasMsg wrapper and use the new sender method). WebGLCommands are the same ones as before. So a big number of lines is already reviewed (just moved to a new place) and almost all the modifications in the DOM follow the same pattern. The conformance tests will help to guarantee that we don't miss anything. Yeah, I agree multiplexing contexts is the way to go. > You still need the Webrender API channel (which in the case of Servo is > another IPC channel), right? > If not, I'd be really curious about how have you managed to get rid of it. > There is a WebRenderAPI channel, but it's only used when a canvas is created or resized in order to send the required webrender_api::ImageKeys I was talking about channel steps per WebGL command. I considered that other channel insignificant for performance. Well, this is hard assuming you need to share textures cross-process, > right? If so, why would we want to do that? In general the content > process shouldn't have access to the GPU. > It'd be nice to have a small explanation about which commands are > executed in which process with your proposed architecture. That's only a fake multiprocess mode. I wanted to validate that we won't need to change the WebGL types used in parts of the code (constellation, pipelinestate, script thread, dom) in the long term. For example, we could make this mode to batch all the WebGLCommands and send them to a separate GPU process using shared memory (still using the same types in all the code). I also used it to test the overhead difference between ipc and mpsc. I don't love to have less error-checking in general, though I understand > the point... In any case, this sounds like the kind of standalone > feature that would be nice to have in its own PR (or commit)... > Also, is there any use case for this for now? I think if we're going to > introduce a cargo feature for "Servo as electron-like back-end" > officially, we should do so separately, and add tests for this. Right now it's also another test to validate the types used across all the parts of the code and measure performace. This mode it's a long term idea to squeeze performance. It doesn't need to land now. I can do it in a separate PR if we decide to use it. We want to talk with the "Synchro" team about all the packaging requirements that we need to publish WebGL/WebVR content in the stores (Google Play, Steam, App Store ...) ;) Right now the SM version is quite old, though there's an attempt to > update it going on[1]. > I could believe that a bunch of the benchmarks are kinda biased because > of this. I remember people opening bugs about the overhead of function calls into > JS, and a bunch of fixes a while ago. That's probably going to change > substantially with the JS engine update though, so you should probably > talk with Nick or Josh about it. That's great news (in the sense of margin for improvement). I remember having nice performance boost when updating VM engines in the past ;) On 22 July 2017 at 19:35, Emilio Cobos Álvarez wrote: > Hi Imanol, > > On 07/21/2017 10:58 PM, Imanol Fernández wrote: > > Hello, > > > > I want to share the ideas, code and some benchmarks of the new Servo > WebGL > > architecture and refactor I'm working on. I'd love to hear some feedback > > about the big picture. For smaller things or nits we could do that in the > > PR review (which I'll plan to do beginning next week if there aren´t > > strong opinions about the overall architecture). > > > > First, I'll mention some of the problems of the current Servo
Re: [dev-servo] New Servo WebGL Architecture Proposal (benchmarks included!)
Hi, this plan looks like a great improvement over the current state of webgl in servo. > * WebGL commands are currently run in the WR backend thread which could > be > bad for UI latency (see https://github.com/servo/webrender/issues/607) Yeah, thanks for getting rid of this! > The connection between a WebGL canvas and WR is implemented via the WR > ExternalImage APIs as Glennw recommended. Great! > The new architecture creates a single thread/process to manage all WebGL > commands from multiple canvas sources. This will reduce the footprint for > creating a new canvas (avoid creating specific thread + ipc-channel) and > reduces the render-path for each WebGL command by using a single "channel > step" Cool, the thread-per-canvas thing is insane. > we may be interested in > batching WebGLCommands, using shared memory, or other channel approaches > to improve performance. You probably already have, but I suggest having a look at what chrome's doing in this are (batching up commands to send to their GPU process instead of sending each command) > * Immediate WebGL calls to a GL context living in the Script thread (I > did > some benchmarks with this enabled (sometimes it's faster running the gl > call than sending it) . I'd like to focus on a thread bases render path > though in order to parallelize JS code better) In-script-thread webgl is also good for latency. Currently Chrome's VR implementation has an extra frame of latency compared to Firefox (which I believe is due to the gl remoting) and it is very noticeable in VR. > * WebGL double buffering: It may be a good idea to improve performance > and > synchronization (e.g. ping pong WebGL render textures each frame, so WR > uses a texture to composite the main context while the WebGL frame is > rendering the next frame to a different texture). This may add some > memory > overhead. We can implement some texture pooling (I think that Firefox > does > that) Yes, Firefox does that. > * How does the SpiderMonkey version used in Servo compare to V8? SpiderMonkey is doing particularly well on the games type of things compared to V8 (for various good and bad reasons). There's also the better support for asmjs. More generally, I'd rather avoid picking the competitor's tech instead of ours unless there is a very strong incentive because of the PR foot-gun that it generates. Cheers, Nical ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
[dev-servo] Unfinished/Not taken up GSOC Projects
Hi, I would like to work on any GSOC servo project not taken up by anyone. Is there any such project left? It will be very helpful if someone can guide me a little. I am a newbie to open source contributions. I have a little coding experience in python, but willing to learn anything in the proccess of completing the project. Thanks in advance. -Dipankar ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] New Servo WebGL Architecture Proposal (benchmarks included!)
Hi, This all sounds very good from a WR perspective, exciting! I'm glad to hear the synchronization objects in the external image callback APIs work well, that's great. I also agree that the remaining flickering is a Servo bug - we shouldn't be doing extra composites when not required just because the mouse moved. We need to fix this at some point, since it causes other issues too. Look forward to seeing the PR and getting this merged! :) Cheers On 22/07/17 06:58, Imanol Fernández wrote: Hello, I want to share the ideas, code and some benchmarks of the new Servo WebGL architecture and refactor I'm working on. I'd love to hear some feedback about the big picture. For smaller things or nits we could do that in the PR review (which I'll plan to do beginning next week if there aren´t strong opinions about the overall architecture). First, I'll mention some of the problems of the current Servo/WebRender WebGL implementation and source code organization * Part of the WebGL implementation is included in WebRender. This makes very slow to make WebGL contributions. For example in order to add a new WebGL method I usually need push a PR to gleam (wait until it's merged and the crate published), then submit another PR to WebRender (wait for the review & merge) and wait again until the new WR is merged into Servo (with low control about it because WR can have in-progress non-related features not ready-to merge) * When adding WebGL related code to WR, the WebGL code is not tested when it lands into WR. It's only tested when a WR update lands into Servo. When a WebGL related test fails in a WR update it's usually fixed by reverting the commit that caused it to avoid delaying the WR update. This makes the testing process difficult. * The current implementation creates a new WebGL renderer thread (with it's ipc-channel) for each canvas created in JavaScript. This can quickly exhaust the file descriptor's available in the system for webpages that create a lot of canvases (e.g Shadertoy) or on games that require many canvases. Creating a canvas instance in JavaScript is slower than it should because of this too. * WebGL commands are currently run in the WR backend thread which could be bad for UI latency (see https://github.com/servo/webrender/issues/607) * Currently there are two ipc-channels steps for each WebGL-call to hit the driver (JS ==> WebGL Render Thread ==> WebRender). This adds a lot of overhead. * Slow compilation times: Modifying a WebGL command code recompiles WebRender_traits which causes additional component recompilations in Servo. * Flickering and shyncronization problems with the WR compositor (I created this issue some time ago: https://github.com/servo/servo/issues/14235) Goals that I want to achieve in the new WebGL architecture design: * Make WebGL development cycle easier. * Minimize the render path overhead for WebGL commands. * Flexible architecture for multiprocess and inprocess scenarios. * A special mode for packaging WebGL applications (not enabled by default, more details later) * Faster compilation times * Proper shyncronization to avoid flickering issues * Compatibility with the WebVR render path Now I'm going into some detail of how I tried to best achieve each stated goal. You can check the "almost ready for PR" source code here: https://github.com/MortimerGoro/servo/commit/c2db42bbbae6d6cb612d0aa2580552a2ff3b0acf (still have to add more docs, do a rebase and clean some tests) Make WebGL development cycle easier. --- This is fixed by splitting the WebGL code into it's own component and removing all WebGL code from WebRender. Additional benefits for WebRender (see https://github.com/servo/webrender/issues/1353): * Simplify the internals of resource cache by removing the GL context management code. * Remove the WebGL cargo feature, since not all clients use it. I opted to move the WebGL code into a Servo component. It eases development, testability and I don't think that there are use cases to use the component outside Servo. Anyway, it's a low coupled component, so it will be easy to move if required but I don't think that it's worth it. The connection between a WebGL canvas and WR is implemented via the WR ExternalImage APIs as Glennw recommended. When the new Architecture Lands in servo, I'll make a separate PR to removes all the unneeded WebGL management code in WR. Minimize the render path overhead for WebGL commands -- The new architecture creates a single thread/process to manage all WebGL commands from multiple canvas sources. This will reduce the footprint for creating a new canvas (avoid creating specific thread + ipc-channel) and reduces the render-path for each WebGL command by using a single "channel step" Flexible architecture for multiprocess and inprocess scenarios
Re: [dev-servo] Categorizing Stylo reftest failures
Now that we've fixed most of these I went through it again and checked stuff. Filtering out scoped style and web components failures we only have a hundred tests left I recategorized most of them: https://gist.github.com/Manishearth/086118c940ff86a6cfc573f53c508279 The major issues right now are first-line and the styloVsGecko / issue (which does not block landing since it's a slight discrepancy between stylo and gecko) -Manish Goregaokar On Thu, May 11, 2017 at 5:19 PM, Manish Goregaokar wrote: > And, it's done! All reftest failures classified. > > Permalink to current revision: > https://gist.github.com/Manishearth/086118c940ff86a6cfc573f53c508279/a181e327a4e2d83d75e28862d1fc01e121923bff > > Bear in mind that the try run this is based on is a few days old so some of > the things there may be fixed in master. I've been making drive-by patches > for many of the things I come across. > > I'll soon do a separate analysis of the HasAuthorSpecifiedRules stuff to see > what's still failing since Matt's patches have landed. There are 913 tests > marked as HasAuthorSpecified in that file, and Matt's patches fixed 530, so > we still have some input widget bugs floating around. > > > I will also be removing now-passing tests if I see them, but don't rely on > that being true. I don't intend to put too much time into maintaining this > list; the main point was to get an idea of what needs to be fixed. I will > try to investigate things which still fail when their corresponding bug > lands. > > > Thanks, > > -Manish Goregaokar > > On Wed, May 10, 2017 at 5:14 PM, Manish Goregaokar > wrote: >> >> So I took a try push with all expectations set as passing and went about >> categorizing the reftest failures. >> >> I'm halfway done. This is pre-HasAuthorSpecifiedRules and there are lots >> of related failures. At some point I'll extract all the HASR-marked failures >> and have a second look at the ones which are still not passing after the >> HASR bug lands. >> >> https://gist.github.com/Manishearth/086118c940ff86a6cfc573f53c508279 >> >> While I've investigated many of these, many others still don't have well >> known reasons. >> >> It would be quite helpful if everyone could help investigate. Ctrl-F >> "unknown" on that page, anything with "unknown" in the explanation needs >> more investigation. >> >> If you do file bugs (or find existing bugs), please let me know so that I >> can update the page. I haven't yet had time to really file bugs for each >> failure, and I intend to do this once done with the rest, but help now would >> be appreciated! >> >> There are also some possibly-low-hanging-fruit failures where I've >> investigated the reason and mentioned it, but not yet filed an issue. >> >> I'll continue to push more reftest failure categorizations as I get >> through, and I intend to write some scripts that let me keep this doc >> somewhat up to date as major groups of failures get fixed. >> >> Thanks, >> -Manish Goregaokar > > ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
