Closed Bug 771720 Opened 10 years ago Closed 10 years ago

Miscellaneous gc/Statistics.cpp improvements

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16
Tracking Status
firefox15 --- fixed

People

(Reporter: billm, Assigned: billm)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
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.
Attachment #639849 - Flags: review?(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.
Attachment #639849 - Flags: review?(terrence) → review+
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.
Target Milestone: --- → mozilla16
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.
Attachment #639849 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/13e12510e60f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 639849 [details] [diff] [review]
patch

[Triage Comment]
Low risk GC perf fix, approved for Aurora 15.
Attachment #639849 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.