1527905 - Switch from zone-per-tab to zone-per-toplevel-load

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

Conceptually, if zones are supposed to represent similar-lifetime objects, zone-per-toplevel-load makes a lot more sense than zone-per-tab.

I want to make this change because it makes bug 1523843 much simpler, and in particular prevents performance and memory regressions due to old objects being kept alive because we shared a compartment across navigations in a single tab.

On its own, this change is a memory regression, but bug 1527742 mitigates most of that, and the combination of the two is a memory win (though a slightly smaller win than just bug 1527742 on its own).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1527905. Switch from zone-per-tab to zone-per-toplevel-load. r=mccr8

The main reason for this change is that we don't really want to share
compartments across same-origin navigations in the same tab (because that will
tend to keep the oldest global involved alive, due to CCWs and
XPCWrappedNatives allocated in that global). We could somehow flag
compartments as not sharable when we navigate, but it's simpler to just switch
zones, since we restrict our search of shareable compartments to a single zone.

A side benefit is that this way the lifetime of objects in a single zone is
more likely to be similar.

Comment 2

[Boris Zbarsky [:bzbarsky]]

ccing some people who might care about GC stuff.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4548b735febd
Switch from zone-per-tab to zone-per-toplevel-load.  r=mccr8

Comment 4

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/4548b735febd

Comment 5

[Ionuț Goldan [:igoldan]]

Performance improvements noticed! \0/

== Change summary for alert #19391 (as of Fri, 15 Feb 2019 00:16:48 GMT) ==

Improvements:

3% raptor-tp6-slides-firefox fcp linux64 pgo 797.08 -> 773.38
3% raptor-tp6-slides-firefox linux64 pgo 931.02 -> 906.75

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=19391

Comment 6

[Boris Zbarsky [:bzbarsky]]

Performance improvements noticed! \0/

Those are from bug 1527742.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Well, actually, I don't know for sure. There should be memory improvements from bug 1527742, but these are performance improvements...

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

This seems to have introduced very high frequency js failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=jsreftest%2Cwindows7-32&fromchange=591ffe90d0edbd8190427f09a14e39a92a327aee&tochange=182057bee133b28a51e32c391ac44dedeb1e0575&selectedJob=229243735

At the time of the landing these tests were not ran, so the failures were not spotted on time.
The changes were merged around to central and inbound.

:bz :jandem :jmaher

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Bogdan Tara[:bogdan_tara] from comment #8)

This seems to have introduced very high frequency js failures:

I'm looking into this FWIW, should have some more data later today.

Comment 10

[Andrew McCreight [:mccr8]]

The failures are an OOM. The patch here shouldn't affect JS shell behavior, which means bug 1527742 is going to be directly at fault. Maybe there's some issue around reusing decommitted memory that that change exposed.

Comment 11

[Jan de Mooij [:jandem]]

(In reply to Andrew McCreight [:mccr8] from comment #10)

The failures are an OOM. The patch here shouldn't affect JS shell behavior, which means bug 1527742 is going to be directly at fault. Maybe there's some issue around reusing decommitted memory that that change exposed.

It's the jsreftests running in the browser..

Comment 12

[Jan de Mooij [:jandem]]

What I'm seeing on Try is that the runtime has 5000 zones when we OOM. I'll look into our poke-GC-on-navigation heuristics..

Comment 13

[Jan de Mooij [:jandem]]

The patch in bug 1529306 seems to fix this on Try:

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=4b1f26f3c1cf4931c159a99c65b1dc603607b862

(Also includes the patch for bug 1529265 but I don't think that one is required for this.)

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

thanks for working on this :jandem, sounds like we are close to a fix.

Comment 15

[Intermittent Failures Robot]

14 failures in 2602 pushes (0.005 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 14

Platform breakdown:

• windows7-32: 13
• android-em-7-0-x86_64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1527905&startday=2019-02-18&endday=2019-02-24&tree=all

Comment 16

[Florin Strugariu [:Bebe]]

== Change summary for alert #19564 (as of Fri, 22 Feb 2019 06:32:34 GMT) ==

Improvements:

5% JS linux64-stylo-sequential opt stylo-sequential 113,904,145.01 -> 108,580,789.31

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=19564

Comment 17

[Boris Zbarsky [:bzbarsky]]

Again, that's from bug 1527742, not this bug.