Closed
Bug 1405445
Opened 4 years ago
Closed 3 years ago
Preallocate the display list buffer to a reasonable size.
Categories
(Core :: Graphics: WebRender, enhancement, P1)
Core
Graphics: WebRender
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox57 | --- | unaffected |
firefox58 | --- | unaffected |
People
(Reporter: jrmuizel, Assigned: jrmuizel)
References
Details
(Whiteboard: [wr-mvp])
Attachments
(1 file)
11.35 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•4 years ago
|
||
Assignee: nobody → jmuizelaar
Attachment #8914873 -
Flags: review?(bugmail)
Comment 2•4 years ago
|
||
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.
Attachment #8914873 -
Flags: review?(bugmail) → review+
Comment 3•4 years ago
|
||
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•4 years ago
|
||
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•4 years ago
|
||
Ok, fair enough.
Updated•4 years ago
|
Whiteboard: [wr-mvp] [triage]
Pushed by jmuizelaar@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0376b0f2075a Preallocate the display list buffer to a reasonable size. r=kats
Updated•4 years ago
|
Updated•4 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [wr-mvp] [triage] → [wr-mvp]
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
Flags: needinfo?(jmuizelaar)
Pushed by jmuizelaar@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/08cf5146eff1 Preallocate the display list buffer to a reasonable size. r=kats
![]() |
||
Comment 9•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/08cf5146eff1
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assignee | ||
Updated•3 years ago
|
Flags: needinfo?(jmuizelaar)
You need to log in
before you can comment on or make changes to this bug.
Description
•