1120271 - Add compacting support to SourceBuffer

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

SourceBuffer uses memory efficiently when images have a Content-Length header, but when it doesn't know the size in advance it generally ends up allocating more memory than it needs because it grows its buffer exponentially. When an image has finished loading and all decoders have finished, we should compact the SourceBuffer so it uses only the memory it needs.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: Add compacting support for SourceBuffer

Timothy, don't feel like you need to review this today. It's ready now so I'm
going ahead and uploading it, though.

This patch adds compacting support for SourceBuffer. It's pretty
straightforward: we count the number of active consumers, and every time we
*both* have zero active consumers and the SourceBuffer is marked complete, we
attempt to compact. Compacting just means allocating a single new chunk that can
hold all of the data, and then copying all of the old chunks into it. We do this
with the lock held so that we don't have to worry about any new consumers coming
along and grabbing pointers to the data.

Comment 2

[Seth Fowler [:seth] [:s2h]]

Attachment: Add compacting support for SourceBuffer

Thanks for the review, Timothy!

This needed a minor rebase.

Comment 3

[Seth Fowler [:seth] [:s2h]]

Here's a try job:

https://tbpl.mozilla.org/?tree=Try&rev=9047b70e2ea0

Comment 4

[Seth Fowler [:seth] [:s2h]]

There are failures in that try job, but they're unexpected passes from bug 985193, which was mistakenly included in that try push.

I went ahead and pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/3c89bb35ffd7

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3c89bb35ffd7

Comment 6

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8549890 [details] [diff] [review]
Add compacting support for SourceBuffer

Approval Request Comment
[Feature/regressing bug #]: Part of fix for bug 1125272 and bug 1119938.
[User impact if declined]: Already approved.
[Describe test coverage new/current, TreeHerder]: In 38 for a long time.
[Risks and why]: Low risk; in 38 for a long time.
[String/UUID change made/needed]: None.

Comment 7

[Seth Fowler [:seth] [:s2h]]

Pushed to Aurora:

https://hg.mozilla.org/releases/mozilla-aurora/rev/e2a6fcce0410

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8549890 [details] [diff] [review]
Add compacting support for SourceBuffer

Aurora approval previously granted for a collection of ImageLib related bugs that Seth worked on and QE verified before uplift.