1047182 - Return early when there's nothing to finalize

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Emanuel Hoogeveen [:ehoogeveen]]

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)

Comment 1

[Emanuel Hoogeveen [:ehoogeveen]]

Attachment: Add early returns to foregroundFinalize and forceFinalizeNow.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=6b5f5f5dbec4

Comment 2

[Emanuel Hoogeveen [:ehoogeveen]]

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 3

[Terrence Cole [:terrence]]

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

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82ec0774f1ac

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/82ec0774f1ac