Closed Bug 1518405 Opened 5 years ago Closed 5 years ago

Enable picture caching

Categories

(Core :: Graphics: WebRender, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- unaffected
firefox66 --- fixed

People

(Reporter: gw, Assigned: gw)

References

Details

(Keywords: feature)

Attachments

(2 files)

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?

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.

Keywords: feature
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All

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
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/95324d36ded5
Turn on picture caching in webrender. r=jrmuizel
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Depends on: 1519064
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!

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

Assignee: kats → nobody
Assignee: nobody → gwatson
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2664998ae6e6
Turn on picture caching in webrender. r=jrmuizel
Priority: P3 → P2
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Severity: normal → enhancement
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: