Return early when there's nothing to finalize

RESOLVED FIXED in mozilla34

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ehoogeveen, Assigned: ehoogeveen)

Tracking

Trunk
mozilla34
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

In bug 1017165 I removed the early return from foregroundFinalize, which then caused bug 1041307. After fixing that bug (which is unnecessary now due to the removal of JS_ION) I started wondering whether putting the early return back would be a good idea, so I decided to do some profiling. Below is the result of running a single pass of MemBench:

                        Calls    % empty before    % empty after
foregroundFinalize()    71132         22.261429         2.524883
backgroundFinalize()   171770          0.000000         0.074518
forceFinalizeNow()     106698         41.359726         0.026242

Here 'Calls' is the total number of calls to that function over the session, '% empty before' is the percentage of times the function was called with an empty list of arenas to sweep, and '% empty after' is the percentage of times all the arenas to sweep were released during finalization (which I separated from '% empty before' by adding early returns).

Returning early isn't a big win - serializing the SortedArenaList shouldn't take long compared to anything else we do during GC. Still, avoiding that extra work 33.72% of the time* (not counting background finalization) seems like a nice win for something so easy to do.

* 33.72% = (71132*22.261429 + 106698*41.359726) / (71132 + 106698)
Try run: https://tbpl.mozilla.org/?tree=Try&rev=6b5f5f5dbec4
Assignee: nobody → emanuel.hoogeveen
Status: NEW → ASSIGNED
Attachment #8465929 - Flags: review?(terrence)
One thing that might be worth mentioning about the number of calls to each function: I incremented the call counter for foregroundFinalize only once per incremental GC (by incrementing the counter only in the early return or after finalization completed), so the total number of calls may be higher.
Comment on attachment 8465929 [details] [diff] [review]
Add early returns to foregroundFinalize and forceFinalizeNow.

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

Good find! r=me
Attachment #8465929 - Flags: review?(terrence) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/82ec0774f1ac
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.