This bug was filed from the Socorro interface and is report bp-f0d21639-644d-4036-ae6e-fe6f32141103. ============================================================= The array in CollectWhite is about 1% of release channel OOMs. It looks like a great candidate for a SegmentedArray.
In IRC, dmajor said about the sizes of these OOM crashes, "roughly 40% between 1-2mb, another 40% between 2-3", "ah so the next 40% is 2-10mb", so the array is getting quite giant. dmajor also said "My goal is just to make sure we can survive when we have only 1mb 'holes' in the address space available". I think we gather telemetry on the values of this in CYCLE_COLLECTOR_COLLECTED. As you can see, mostly they are very small, so it would be nice to not use a ton of memory when we don't have to. The CC also runs per-worker. The telemetry there is CYCLE_COLLECTOR_WORKER_COLLECTED. The current implementation is |nsAutoTArray<PtrInfo*, 4000>|. All we do with this array is push things on it, then iterate over it a few times, calling methods on it.
I can take this. It's similar to bug 1094564.
Assignee: dmajor → n.nethercote
Here's some data from running http://gregor-wagner.com/tmp/mem50. Each lines shows the length and capacity. Almost all of them are less than 10% full.
I recently changed this so we only put in C++ objects, not JS ones (which we just run no-ops on), which is probably why they are often very empty now.
This patch does the following. - Moves the logic for computing the ideal capacity for a SegmentedArray out of SnowWhiteKiller into its own class, SegmentedArrayCapacity. - Adds SegmentedArray::Length(). - Replaces the nsTArray in CollectWhite(), which can be very large, with a SegmentedArray.
Attachment #8520526 - Flags: review?(bugs)
Comment on attachment 8520526 [details] [diff] [review] Use a SegmentedArray in nsCycleCollector::CollectWhite Don't understand '// njn: resize' comment. Drop it or change it to something better. I would just not add the Length() and move uint32_t numWhiteNodes; to be above uint32_t numWhiteGCed = 0; (and initialize numWhiteNodes to 0) and then ++numWhiteNodes whenever we do whiteNodes.AppendElement(pinfo); With those, r+.
Attachment #8520526 - Flags: review?(bugs) → review+
> Don't understand '// njn: resize' comment. > Drop it or change it to something better. Whoops. I write comments like that as notes to myself for things that need fixing before review or landing. I forgot to remove this one. > I would just not add the Length() and > move uint32_t numWhiteNodes; to be above uint32_t numWhiteGCed = 0; > (and initialize numWhiteNodes to 0) > and then ++numWhiteNodes whenever we do whiteNodes.AppendElement(pinfo); Will do. Thank you for the quick review.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.