1642495 - Lots of time spent in buffer_data_untyped|gleGetFreeOrphanNode on macOS

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

https://share.firefox.dev/2ZXsHV1

This happened while scrolling a zoomed in hackernews page.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Looking at a profile from Instruments it looks like we're spending a lot of time looping traversing a hashtable/linked list in gleGetFreeOrphanNode

Comment 2

[Jeff Muizelaar [:jrmuizel]]

In 10.15 the buffer recycling logic looks something like:

if (size > 0x20000) {
   round size up to nearest 0x1000
} else {
   bucket into [0x1000, 0x2000, 0x4000, 0x8000, 0x10000, 0x20000]
}

do a hash lookup based on the bucketed size and look for an orphaned bucket that matches.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

for 72 calls to buffer_data in one frame we go around the search loop 3426 times.

Comment 4

[Dzmitry Malyshau [:kvark]]

We should have a define like MAX_INSTANCE_BUFFER_SIZE in renderer.rs, compute the maximum number of instances from it, and chunk the instance vectors accordingly. This is easy to do, cheap to do, and will make sure we never hit the bad case on macOS.

Comment 5

[Dzmitry Malyshau [:kvark]]

Attachment: Limit WebRender instance buffer sizes

This is an attempt to improve our relationship with the drivers.
Currently, we work with the instance buffer limit dictated by the macOS drivers.
We can consider lowering it, since it will only make the driver work eaiser.

Comment 6

[Dzmitry Malyshau [:kvark]]

I have a feeling this isn't going to solve HN, since it's unlikely hitting the instance limit anyway.
Perhaps, we can test the build before going forward with it?

Comment 7

[Jeff Muizelaar [:jrmuizel]]

I see some large number of draw calls when zooming, I suspect there might be some badness here where the amount of time we spend looking for an orphaned buffer is proportional to the number of draw calls that we're doing because we end up with that many buffers.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

(In reply to Dzmitry Malyshau [:kvark] from comment #6)

I have a feeling this isn't going to solve HN, since it's unlikely hitting the instance limit anyway.
Perhaps, we can test the build before going forward with it?

Yeah, I think it's actually the small buffers causing the problem not the big ones.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

It looks like the reason we don't find a match with the orphaned buffers is because mostly because the usage doesn't match.
We're looking for GL_STREAM_DRAW and at least some of the orphaned buffers are GL_DYNAMIC_DRAW.

Comment 10

[Dzmitry Malyshau [:kvark]]

Attachment: Use the same usage hint in WebRender for one-time reset

This should let the GL drivers to re-use the PBOs more aggressively,
and traverse the orphan list less.
Fwiw, it doesn't look like Angle differentiates between StreamDraw and DynamicDraw:
https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/gfx/angle/checkout/src/libANGLE/renderer/d3d/BufferD3D.cpp#65-66

Comment 11

[Dzmitry Malyshau [:kvark]]

Attachment: Switch all WebRender HW-accelerated GPU cache updates to Scatter

scattered GPU updates use data transfers most efficiently, since
they need a single slice of a buffer to do all the updates per frame, instead
of uploading each small section of a row independently.

Comment 12

[Jeff Muizelaar [:jrmuizel]]

10.15 also has a hard coded maximum combined size of orphaned buffers of 0x4000000 bytes (~67MB) and will free orphans until it gets below that max.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9f33b1f23a0
Limit WebRender instance buffer sizes r=gw

Comment 14

[Jeff Muizelaar [:jrmuizel]]

The usage hint definitely improves things. gleGetFreeOrphanNode is the hotest function before and is much lower after.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec174e49592b
Use the same usage hint in WebRender for one-time reset r=gw

Comment 16

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/a9f33b1f23a0

Comment 17

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/ec174e49592b

Comment 18

[Markus Stange [:mstange]]

See also bug 1645716 for some more discussion on this problem.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7abdf136d365
Switch all WebRender HW-accelerated GPU cache updates to Scatter r=gw

Comment 20

[Narcis Beleuzu [:NarcisB]]

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

Comment 21

[Dzmitry Malyshau [:kvark]]

I think the patches improved the situation. Ultimately, we need to move to a world when PBOs are allocated in fixed-size chunks, and in less amount than today. This would make them trivially reusable (by either us or the driver), and stress the user-mode driver side less.

Comment 22

[Kris Taeleman (:ktaeleman)]

@kvark: can you close this bug and create a followup bug instead?

Comment 23

[Dzmitry Malyshau [:kvark]]

That other bug is already on file -
https://bugzilla.mozilla.org/show_bug.cgi?id=1602550