Investigate if the background free is still necessary

RESOLVED FIXED in mozilla34

Status

()

defect
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: igor, Assigned: terrence)

Tracking

(Blocks 1 bug)

Other Branch
mozilla34
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

As the background finalization removes the need in most of the background free calls, we should investigate if we can remove the support for them and make JSContext::free_ to be an alias to js_free.
The patch measures how often cx->free_ in fact does the background free. It shows the following:

1. During v8 the max ration of delayed_cx_free/total_cx_free is 2.7% with absolute number of the delayed background free calls staying under 300.

2. When executing gregor-wagner.com/tmp/mem benchmark that opens 150+ tabs with various pages and then closing them the long-term average ratio becomes about 12%.

3. When all the tabs from the above benchmark are closed using the button in the benchmark, that results in the peak efficiency of the background free at about 80% that pushed the long term-average to 18%. Note also that in most cases the peaks are triggered by the idle GC when the GC pause much less observable.

From this it looks that with the background finalization implemented the explicit background free is no longer necessary.
https://tbpl.mozilla.org/?tree=Try&rev=e1327976acc3
https://tbpl.mozilla.org/?tree=Try&rev=fb52e6b80d13

Even if the background free is heavily used, the use of per-thread malloc heaps means that this may not even be lower-latency if the off-thread free needs to take a lock.
Assignee: igor → terrence
Status: NEW → ASSIGNED
Attachment #8467905 - Flags: review?(jcoppeard)
Comment on attachment 8467905 [details] [diff] [review]
remove_background_free-v0.diff

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

I'd be happy to see this go.
Attachment #8467905 - Flags: review?(jcoppeard) → review+
Whoops, bustage under the prior try runs:
https://tbpl.mozilla.org/?tree=Try&rev=020859802a6c
No obvious regression, let's get this on AWFY to verify.

https://hg.mozilla.org/integration/mozilla-inbound/rev/eab45b9fa851
https://hg.mozilla.org/mozilla-central/rev/eab45b9fa851
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.