1405445 - Preallocate the display list buffer to a reasonable size.

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P1)

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: Preallocate the display list buffer to a reasonable size

Comment 2

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

Comment on attachment 8914873 [details] [diff] [review]
Preallocate the display list buffer to a reasonable size

Review of attachment 8914873 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/webrender_bindings/src/bindings.rs
@@ +1077,3 @@
>          WebRenderFrameBuilder {
>              root_pipeline_id: root_pipeline_id,
> +            dl_builder: webrender_api::DisplayListBuilder::with_capacity(root_pipeline_id, content_size, capacity),

I'd slightly prefer if we used the old ::new function instead of ::with_capacity in the case where capacity==0 here. It seems not so great to explicitly initialize the vec with zero capacity for the cases where we pass 0, because those cases really mean "we don't know what size to use", not "we will actually never put anything in here". If we start with zero capacity for the former cases it seems like we're going to do extra reallocations when we could just start with a small-but-nonzero capacity and save some reallocs.

Of course this change would only help if we modified the upstream DisplayListBuilder::new impl to not pass 0 to with_capacity, but instead pass some small-but-nonzero default value.

I don't feel strongly about this, leaving it to you.

Comment 3

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

Also, if you're going to land this soonish, please land it on autoland because otherwise it will conflict with my layers-full deletion patches.

Comment 4

[Alexis Beingessner [:Gankra]]

By convention ::new() is expected to behave ~identically to ::with_capacity(0). I think if we want to do minimum sizing we should handle that in the client, since gecko/servo don't necessarily agree on the issue.

Comment 5

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

Ok, fair enough.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0376b0f2075a
Preallocate the display list buffer to a reasonable size. r=kats

Comment 7

[Wes Kocher (:KWierso)]

This conflicted with my merge to m-c (which included changes from bug 1391816, touching a lot of similar files), so I backed this out to get things merged. Feel free to rebase and reland whenever.

Backout: https://hg.mozilla.org/mozilla-central/rev/086a9a00753e466752c14a255c1ab2a9ddb25b15

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/08cf5146eff1
Preallocate the display list buffer to a reasonable size. r=kats

Comment 9

[Sebastian Hengst [:aryx] (needinfo on intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/08cf5146eff1