Jeff Muizelaar [:jrmuizel] Assignee
	Description • 6 years ago

I'm seeing an appreciable amount of time being spent in ANGLE during texture upload.

https://perfht.ml/2v3zmvx - a profile from the bottom of https://creativecluster.lu

	Jeff Muizelaar [:jrmuizel] Assignee
	Comment 1 • 6 years ago

It looks like this might just be a necessary copy. There may not be anything we can about this without switching to gfx-rs which gives us better control over upload (i.e. we can upload on a different thread)

	Dzmitry Malyshau [:kvark]
	Comment 2 • 6 years ago

In an ideal world, the blob image renderer should be given the space to fill (as opposed to owning that space), and it's going to come from a mapped staging resource owned by the renderer. This would allow us to avoid an extra copy.

	Dzmitry Malyshau [:kvark]
	Comment 3 • 6 years ago

Added a link to upstream issue. I've been looking at it more from the perspective of Gecko, and I came to the following conclusion:
  1. Angle logic appears to be correct. It keeps a staging texture, maps it, copies it, and has a heuristic on when it's best to keep it around instead of re-creating.
  2. Gecko appears to be wrong: it constantly re-creates the blob image every frame. This forces Angle to create a staging resource each frame, and also makes the `Map` call slow, since the associated memory pages need to be initialized to zeroes.

Andrew, is this expected for a blob image to be re-created all the time? I was under impression that Gecko now re-uses the same image for blobs.

	Jeff Muizelaar [:jrmuizel] Assignee
	Comment 4 • 6 years ago

I believe on this site the blob images are changing in size which explains why we're recreating them. On digitalocean.com I don't think they should be changing in size.

	Dzmitry Malyshau [:kvark]
	Comment 5 • 6 years ago

Jeff, on "digitalocean.com" I'm only see it entering the createStagingTexture() when the size changes, and it does appear to be changing when scrolling. So the reason for uploads being slow is the same.

Speaking of which, the logic of maintaining the staging area could be much more efficient. There is no requirement to have the staging texture with the same size. It only needs to have the same pixel format. So the most efficient approach would be to have a pool of staging textures of fixed size per format (e.g. 2Kx2K). When the user wants to update a sub-rect, the implementation would pick a rect from one of the pooled staging textures, map it, fill it, and then issue a copy from it.

Of course this is more complicated than Angle wants it to be, so I wonder if we can force this behavior by changing the logic on our side.

	Dzmitry Malyshau [:kvark]
	Comment 6 • 6 years ago

I think the proper solution for us, one that doesn't involve dancing around Angle-D3D interop, would be to make sure the image resources (especially blobs) live as long as possible. Make them more persistent, to keep the associated data alive across Gecko - WR - Angle line.

	Jeff Muizelaar [:jrmuizel] Assignee
	Comment 7 • 6 years ago

So for large images (where upload performance matters) we should be tiling the blob images. This means that all/most of the images should be 256 byte aligned and not changing in size. I haven't confirmed that digitalocean.com is using tiled blob images but if it's not it sounds like we should make it do so.

	Maire Reavy [:mreavy]
	Comment 8 • 6 years ago

Jeff -- How bad is this now?  How often will a user (likely) hit this?

	Jeff Muizelaar [:jrmuizel] Assignee
	Comment 9 • 6 years ago

I'm worried that this isn't working great still, but don't have any concrete instances right now. Let's see where we are after blob-re-coord and go from there.