Use a SegmentedArray in nsCycleCollector::CollectWhite

RESOLVED FIXED in mozilla36

Status

()

defect
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dmajor, Assigned: njn)

Tracking

({crash})

36 Branch
mozilla36
x86
Windows NT
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2], crash signature)

Attachments

(2 attachments)

Reporter

Description

5 years ago
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.
Assignee

Comment 2

5 years ago
I can take this. It's similar to bug 1094564.
Assignee: dmajor → n.nethercote
Assignee

Comment 3

5 years ago
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.
Assignee

Comment 5

5 years ago
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+
Assignee

Comment 8

5 years ago
> 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.
Assignee

Updated

5 years ago
Whiteboard: [MemShrink] → [MemShrink:P2]
https://hg.mozilla.org/mozilla-central/rev/91e6e73aa5e6
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.