771720 - Miscellaneous gc/Statistics.cpp improvements

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

The biggest improvement here is that I discovered that the page fault counts in bug 745034 are actually pretty expensive to collect. Now that we have ~1000 compartments, we do beginPhase and endPhase maybe 15,000 times in a single GC. That means 15,000 calls to getrusage, which isn't terribly fast. In a large browsing session, this was adding tens of milliseconds to the sweep phase of every GC.

Additional changes:

1. Split the discard_code phase into one that happens during marking and one that happens during sweeping. If these aren't separate, it's hard to know how much of the mark phase or the sweep phase are accounted for by sub-phases.

2. Split the mark_other phase into mark_weak, mark_gray, and mark_gray_and_weak. I've noticed that this phase takes a long time if your browser has been open for a while, so we should try to understand which component is slow.

3. Add new sweep_atoms and sweep_tables phases. I was working on figuring out where we're spending time, and these two end up being significant.

4. Add a free_ti_arena phase. This one is also a significant time sink. Note that I had to add the oldAlloc.freeAll() call in order to account for this time. Previously, this was happening in the destructor for oldAlloc, and it's really hard to account for the time taken by a destructor. The semantics of the code should be unchanged, aside from timing.

Comment 1

[Terrence Cole [:terrence]]

Comment on attachment 639849 [details] [diff] [review]
patch

Review of attachment 639849 [details] [diff] [review]:
-----------------------------------------------------------------

> In a large browsing session, this was adding tens of milliseconds to the sweep phase of every GC.

!?!?

Wow, good catch.

I'd be interested to know what the overhead of the PRMJ_Now() are as well.  There's this really appalling "optimization" at the front of the windows implementation to "keep from regressing startup time".  There are only 3 calls to PRMJ_Now() while starting up -- gcstats is by far the largest user in the shell now and it would be nice if we could kill half of prmjtime.cpp.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/13e12510e60f

On Linux, PRMJ_Now() is pretty fast. We spend at most a few milliseconds doing it, even with thousands of compartments. I don't know what sort of performance it gets on Windows, though.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 639849 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 745034
User impact if declined: GCs may needlessly take a long time.
Testing completed (on m-c, etc.): On m-c.
Risk to taking this patch (and alternatives if risky): Low. It only affects GC data collection, which shouldn't affect browser behavior at all.
String or UUID changes made by this patch: None.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/13e12510e60f

Comment 5

[Alex Keybl [:akeybl]]

Comment on attachment 639849 [details] [diff] [review]
patch

[Triage Comment]
Low risk GC perf fix, approved for Aurora 15.

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/468ab3179e4a