1091986 - Change nsStreamLoader::mData to a mozilla::Vector.

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Nicholas Nethercote [inactive]]

nsStreamLoader has a buffer which gets grown in the smallest possible steps.
This means we sometimes see bad growth sequences like this:

> net: 9727 -> 10315
> net: 10315 -> 11299
> net: 11299 -> 14452
> net: 14452 -> 15438
> net: 15438 -> 18591
> net: 18591 -> 18782
> net: 18782 -> 21935
> net: 21935 -> 23496
> net: 23496 -> 26649
> net: 26649 -> 28395

A doubling growth strategy is typically used for growable buffers.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Remove an over-zealous assertion in Vector.h

Vector.h has several assertions involving CapacityHasExcessSpace(). These
basically check that the heap storage is the size we expect based on our
doubling algorithm.

However, if you use initCapacity(), you can end up with a heap buffer that's
not a standard size, and you can then trigger this assertion. So it should just
be removed. The other similar assertions remain, because they are on paths that
should only be reached once doubling has taken place.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Change nsStreamLoader::mData to a mozilla::Vector

This patch converts nsStreamLoader::mData to a mozilla::Vector, which uses a
doubling growth strategy. (Note that the initial size of mData will be exactly
that requested; the doubling only kicks when a buffer is resized.)

Loading Gmail, TechCrunch and CNN, this reduces the heap churn from 7.5 to 7.2
MiB and reduces the number of reallocations from 126 to 82. It will also help
with heap fragmentation by avoiding unusual allocation sizes; see bug 1048044
comment 35 for a similar case.

It also makes the code more concise and less error-prone, by avoiding the
manual buffer management. And because mozilla::Vector uses size_t for its
length, we can avoid some checking code involving UINT32_MAX.

Comment 3

[Patrick McManus [:mcmanus]]

Comment on attachment 8514748 [details] [diff] [review]
(part 2) - Change nsStreamLoader::mData to a mozilla::Vector

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

awesome. thanks.

Comment 4

[Nicholas Nethercote [inactive]]

I had to tweak the extractRawBuffer part slightly to get past some test failures. The new version looks good on try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=74081468c6f8

Comment 5

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b24680cc584c
https://hg.mozilla.org/integration/mozilla-inbound/rev/4be4cf0afc29

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b24680cc584c
https://hg.mozilla.org/mozilla-central/rev/4be4cf0afc29