Closed Bug 1413546 Opened 7 years ago Closed 5 years ago

Enable retained display lists for parent process

Categories

(Core :: Web Painting, enhancement, P2)

Product:
Core
Component:
Web Painting
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla69
Tracking Flags:
Tracking Status
firefox69 --- fixed

People

(Reporter: mikokm, Assigned: mikokm)

References

(Depends on 1 open bug, Blocks 1 open bug, Regressed 1 open bug)

Details

Attachments

(3 files)

Add a pref to enable retained-dl for chrome
(deleted), patch
bugs
: review+
Details | Diff | Splinter Review
Bug 1413546 - Fix e10s logic in nsLayoutUtils::AreRetainedDisplayListsEnabled() and set layout.display-list.retain.chrome to true
(deleted), text/x-phabricator-request
mattwoodrow
: review+
Details
Bug 1413546 - Enable retained display lists for parent process r=mattwoodrow
(deleted), text/x-phabricator-request
Details
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: 6 years ago
Keywords: leave-open
Resolution: --- → FIXED
Depends on: 1488889
status-firefox64: --- → fixed
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
status-firefox64: fixed → ---
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
Depends on: 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

https://hg.mozilla.org/mozilla-central/rev/87409f291fa6

Status: NEW → RESOLVED
Closed: 6 years ago → 5 years ago
status-firefox69: --- → fixed
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
Regressions: 1601306
Regressions: 1639184
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: