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)
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+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.