Re: [dev-servo] Standalone NDK concerns?

[Imanol Fernández]

We talked a bit about standalone NDKs when we worked on adding support for
new android CPU architectures here:
https://github.com/servo/servo/issues/11921#issuecomment-299052122

The benefit of using a standalone NDK is that we have to configure less CXX
flags in Servo. But the main concerns are that is another step and failure
point for people building for Android. Also we have to create different
standalone NDKs for each architecture: armv7, arm64, x86, etc which can
increase build sizes and steps for the user.

IMO is better that we assume the CXX flags set up complexity and only make
the users to set up the PATH to the default android-ndk folder. At least
that's the usual way to compile android libraries. In fact, I had never
heard of standalone NDKs after a lot of time working & compiling libs for
Android until I worked on that Servo integration.


On 7 March 2018 at 06:42, Matt Brubeck  wrote:

> We used to use a standalone toolchain:
> https://github.com/servo/servo/wiki/Building-for-Android/
> 16f371c68d7da393d178646dcb18dac2c2ea8826
>
> but this changed back in 2016; I'm not clear on the exact reasons:
> https://github.com/servo/servo/pull/13945
>
> On Tue, Mar 6, 2018 at 4:46 PM, Josh Bowman-Matthews <
> j...@joshmatthews.net>
> wrote:
>
> > I'm trying to update our NDK from 12b to a more modern 16b. Since the
> > default NDK installation is so convoluted in terms of include/lib/bin
> > directories, I'm giving the standalone SDK a shot and it appears to be
> much
> > more straightforward to integrate into the build. Is there any reason we
> > haven't required the use of standalone SDKs before?
> >
> > Cheers,
> > Josh
> > ___
> > dev-servo mailing list
> > dev-servo@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-servo
> >
> ___
> dev-servo mailing list
> dev-servo@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-servo
>
___
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!)

[Imanol Fernández]

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 <emi...@crisal.io> 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 rev

[dev-servo] New Servo WebGL Architecture Proposal (benchmarks included!)

[Imanol Fernández]

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
--

One of the things I don't like about the current implementation is that it
uses a IpcSender instance as the entry point to send the
WebGL commands from the Script Thread to the renderer in many parts of the
source code. IMO this is not very flexible because we may be interested in
batching WebGLCommands, using shared memory, or other channel approaches to
improve performance.

A WebGL demo can easily send e.g 200 channels messages per frame (doubled
in VR mode) and