Closed Bug 1091986 Opened 6 years ago Closed 6 years ago
Stream Loader::m Data to a mozilla::Vector .
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)
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: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=74081468c6f8
Status: ASSIGNED → RESOLVED
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.