1096624 - Use a SegmentedArray in nsCycleCollector::CollectWhite

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[(Away)]

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.

Comment 1

[Andrew McCreight [:mccr8]]

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.

Comment 2

[Nicholas Nethercote [inactive]]

I can take this. It's similar to bug 1094564.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: data from a mem50 run

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.

Comment 4

[Andrew McCreight [:mccr8]]

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.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Use a SegmentedArray in nsCycleCollector::CollectWhite

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.

Comment 6

[Nicholas Nethercote [inactive]]

Try looks good: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=20c19fd2e1f2

Comment 7

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

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+.

Comment 8

[Nicholas Nethercote [inactive]]

> 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.

Comment 9

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/91e6e73aa5e6

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/91e6e73aa5e6