Miscellaneous gc/Statistics.cpp improvements

RESOLVED FIXED in Firefox 15



JavaScript Engine
5 years ago
5 years ago


(Reporter: billm, Assigned: billm)



Firefox Tracking Flags

(firefox15 fixed)



(1 attachment)



5 years ago
Created attachment 639849 [details] [diff] [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]

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+

Comment 2

5 years ago

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 3

5 years ago
Comment on attachment 639849 [details] [diff] [review]

[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?
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 5

5 years ago
Comment on attachment 639849 [details] [diff] [review]

[Triage Comment]
Low risk GC perf fix, approved for Aurora 15.
Attachment #639849 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 6

5 years ago


5 years ago
status-firefox15: --- → fixed
You need to log in before you can comment on or make changes to this bug.