Closed Bug 1413546 Opened 2 years ago Closed 6 months ago

Enable retained display lists for parent process

Categories

(Core :: Web Painting, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: miko, Assigned: miko)

References

(Depends on 1 open bug, Blocks 2 open bugs, Regressed 3 open bugs)

Details

Attachments

(3 files)

At the moment we are only using display list retaining in content processes.

Enabling retained display lists for parent process crashes when using popups or hovering over some UI elements.

This bug tracks fixing those problems and enabling retained display lists for parent process.
Miko: can you land a patch to turn this into a pref? Thx!
https://searchfox.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp#3671
Flags: needinfo?(mikokm)
Flags: needinfo?(mikokm) → needinfo?(matt.woodrow)
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
Priority: -- → P2
Comment on attachment 8937554 [details] [diff] [review]
Add a pref to enable retained-dl for chrome

layout.display-list.retain.chrome

LGTM. Thx! r+
Attachment #8937554 - Flags: review?(bugs) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/26ee6030db34
Add pref to allow retained display lists within the parent process. r=jet
Depends on: 1427558
Depends on: 1427476
Could we temporarily revert this patch, so it can be improved/less buggy?
Alice, is there a specific reason you marked this as a blocker instead of bug 1344971?
Flags: needinfo?(alice0775)
(In reply to Matt Woodrow (:mattwoodrow) from comment #7)
> Alice, is there a specific reason you marked this as a blocker instead of
> bug 1344971?

no.
No longer depends on: 1427558
Flags: needinfo?(alice0775)
Comment on attachment 8998627 [details]
Bug 1413546 - Fix e10s logic in nsLayoutUtils::AreRetainedDisplayListsEnabled() and set layout.display-list.retain.chrome to true

Matt Woodrow (:mattwoodrow) has approved the revision.
Attachment #8998627 - Flags: review+
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b4b3be038ab8
Fix e10s logic in nsLayoutUtils::AreRetainedDisplayListsEnabled() and set layout.display-list.retain.chrome to true r=mattwoodrow
Backed out for failing win  testing\firefox-ui\tests\functional\sessionstore\test_restore_windows_after_windows_shutdown.py 

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b4b3be038ab8e3f6c8aa4c3bcfdba2e581d65a13

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=192935582&repo=autoland&lineNumber=64571

Backout: https://hg.mozilla.org/integration/autoland/rev/3202d623ead82c71bb1790ecfc5c3d99493d2e1e
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(matt.woodrow) → needinfo?(mikokm)
It seems that test_restore_windows_after_windows_shutdown.py is failing because we have a dangling pointer to RenderFrameParent in nsDisplayRemote. The cause for this is either a missing invalidation for nsDisplayRemote, or a bug in the test that uses a special shutdown mechanic.
Flags: needinfo?(mikokm)
It's possible that there is something up with the test, but I'd expect the same issue to come up with a ordinary OS shutdown, this is afaik the only test that exercises that shutdown path.
Assignee: matt.woodrow → mikokm
Status: NEW → ASSIGNED
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/716d49302a28
Fix e10s logic in nsLayoutUtils::AreRetainedDisplayListsEnabled() and set layout.display-list.retain.chrome to true r=mattwoodrow
Status: ASSIGNED → RESOLVED
Closed: Last year
Keywords: leave-open
Resolution: --- → FIXED
Depends on: 1488889
Target Milestone: --- → mozilla64
Depends on: 1489184
Depends on: 1494124

Should we reopen this given that we have since disabled it?

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → NEW
Target Milestone: mozilla64 → ---
Depends on: 1496491

This was disabled in bug 1496491, because a Windows specific bug with dragging regions prevented users from closing tabs.

Depends on: 1552104
Depends on: 1554373
See Also: → 1554985
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/87409f291fa6
Enable retained display lists for parent process r=mattwoodrow
No longer depends on: 1554985
Status: NEW → RESOLVED
Closed: Last year6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Regressions: 1557761

We noticed this performance improvements:
== Change summary for alert #21276 (as of Tue, 04 Jun 2019 07:38:06 GMT) ==

Improvements:

5% tart windows10-64-shippable opt e10s stylo 2.68 -> 2.53
4% tart windows7-32-shippable opt e10s stylo 2.65 -> 2.54

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

Regressions: 1559706
No longer regressions: 1559706
Regressions: 1559706
No longer regressions: 1559706

Hi, the improvements below came with your patch:
== Change summary for alert #21276 (as of Tue, 04 Jun 2019 07:38:06 GMT) ==

Improvements:

5% tart windows10-64-shippable opt e10s stylo 2.68 -> 2.53
4% tart windows7-32-shippable opt e10s stylo 2.65 -> 2.54
2% tart linux64-shippable opt e10s stylo 2.04 -> 2.00

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

Regressions: 1562184
Regressions: 1559706
Regressions: 1565225
Regressions: 1565922
Regressions: 1490571
You need to log in before you can comment on or make changes to this bug.