Closed Bug 1091986 Opened 6 years ago Closed 6 years ago

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


(Core :: Networking, defect)

Not set





(Reporter: njn, Assigned: njn)




(2 files)

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.
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.
Attachment #8514747 - Flags: review?(luke)
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.
Attachment #8514748 - Flags: review?(mcmanus)
Attachment #8514747 - Flags: review?(luke) → review+
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.
Attachment #8514748 - Flags: review?(mcmanus) → review+
I had to tweak the extractRawBuffer part slightly to get past some test failures. The new version looks good on try:
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.