Enable picture caching

RESOLVED FIXED in Firefox 66

Status

()

P2
enhancement
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: gw, Assigned: gw)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {feature})

unspecified
mozilla66
feature
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64 unaffected, firefox65 unaffected, firefox66 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 months ago

There are still some performance tidy ups, and at least one (seemingly) very rare known visual bug.

But I think we should enable it by default on nightly, and see what breaks and what effect it has on telemetry numbers. We can always disable it if there is any major breakage or performance regressions encountered.

We should make sure the most recent patches in WR (https://github.com/servo/webrender/pull/3482) have landed in m-c before enabling - there is one patch open for review that still needs to be merged first.

I'm not sure the best way to enable it - would flipping the default value of the picture caching preference be right?

(Assignee)

Comment 1

2 months ago

Though, in light of https://bugzilla.mozilla.org/show_bug.cgi?id=1518406 and https://bugzilla.mozilla.org/show_bug.cgi?id=1518407 being reported, perhaps I should investigate those and add the functionality to skip caching tiles that are constantly changing before enabling? That should be a minor change, < 0.5 days work.

Blocks: 1386669, 1494775
status-firefox64: --- → unaffected
status-firefox65: --- → unaffected
status-firefox66: --- → affected
status-firefox-esr60: --- → unaffected
Keywords: feature
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All
(Assignee)

Comment 2

2 months ago

I opened a pull request to only start caching tiles when they have had the same content for > 2 frames, which should (hopefully) resolve the issues reported above, although I can't reproduce a regression of that significance.

So, the pending patches we need to get into m-c before enabling picture caching are:

https://github.com/servo/webrender/pull/3487
https://github.com/servo/webrender/pull/3482

(In reply to Glenn Watson [:gw] from comment #0)

I'm not sure the best way to enable it - would flipping the default value of the picture caching preference be right?

Yeah, changing it at https://searchfox.org/mozilla-central/rev/c3ebaf6de2d481c262c04bb9657eaf76bf47e2ac/modules/libpref/init/all.js#960 would be the way to do it.

Assignee: nobody → kats

Comment 5

2 months ago
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/95324d36ded5
Turn on picture caching in webrender. r=jrmuizel

Comment 6

2 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox66: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

https://hg.mozilla.org/integration/mozilla-inbound/rev/340d5146c405

I backed this out for the talos regreesion, and because we suspect it of causing telemetry regressions.

There are more fixes in-flight, so we'll land it again early next week!

status-firefox66: fixed → affected

Unassigning since I will be away when we want to reland this.

Assignee: kats → nobody
(Assignee)

Updated

2 months ago
Assignee: nobody → gwatson

Comment 10

2 months ago
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2664998ae6e6
Turn on picture caching in webrender. r=jrmuizel
Priority: P3 → P2

Comment 11

2 months ago
bugherder
Status: REOPENED → RESOLVED
Last Resolved: 2 months ago2 months ago
status-firefox66: affected → fixed
Resolution: --- → FIXED
Severity: normal → enhancement
You need to log in before you can comment on or make changes to this bug.