Closed Bug 1362115 Opened 3 years ago Closed 2 years ago

[meta] Turn on gfx.webrender.blob-images (blob images) by default

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox56 --- unaffected
firefox57 --- unaffected
firefox60 --- fixed

People

(Reporter: jrmuizel, Assigned: Gankra)

References

(Depends on 1 open bug)

Details

(Keywords: meta, Whiteboard: [gfx-noted])

Attachments

(3 files, 1 obsolete file)

No description provided.
Depends on: 1362117
Depends on: 1362221
The latest reftest run https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e5e0be482d78f961829bff13bb92b24dfa74ea5&selectedJob=96747316 with fixes for bug 1362117, and bug 1362221. I have looked at any of the failures yet, so they're all free for the taking.
Blocks: 1362245
Whiteboard: [gfx-noted]
Depends on: 1364241
Depends on: 1364626
Depends on: 1364627
Depends on: 1364628
Depends on: 1364629
Depends on: 1365358
Depends on: 1374900
Priority: P3 → P2
Whiteboard: [gfx-noted] → [wr-mvp] [gfx-noted]
Keywords: meta
Priority: P2 → --
Summary: Turn on BlobImage by default → [meta] Turn on BlobImage by default
Whiteboard: [wr-mvp] [gfx-noted] → [gfx-noted]
Looks like we're consistently leaking a handful of NativeFontResourceFontconfig and UnscaledFontFontconfig instances, causing all the debug jobs to fail. Based on the way those objects are allocated and used I'm guessing that we're not deleting all the font stuff during teardown, which leaves some UnscaledFontFontconfig lying around in sFontDataTable. Those objects in turn keep a reference to the NativeFontResourceFontconfig and so those get leaked too.

Other than that there's a lot of reftest failures. And what look like OOMs in the crashtest run.
Depends on: 1431211
Here's a push Lee did with the fix from bug 1431211: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0427966c52e94b61ed74e1b4cf899dc7818ead34&group_state=expanded

The R7/R8 failures can be dealt with by annotating, but there's also a crashtest failure now which looks like an OOM. I've retriggered to see if it's consistent.
The crashtest failure seems pretty consistent, and it's not happening on m-c/inbound/autoland now, so it's almost certainly a result of turning on blob images. We'll need to investigate that.
Making the title more searchable
Summary: [meta] Turn on BlobImage by default → [meta] Turn on gfx.webrender.blob-images (blob images) by default
Assignee: nobody → a.beingessner
The last issue was a crashtest that produces an empty video with a massive size. Webrender blobs this, and because it doesn't know the blob is actually empty it tries to produce tiles. However this ends up requiring 1,000,000,000 tiles which makes us run out of memory and die producing tile metadata.

The fix I take here is to just consider empty videos to have no content (which matches the logic in BuildLayer).


The core issue should still be fixed in webrender, and an issue has been filed, but we can proceed with CI without it.

try build with no crashes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ddf9e53d5c22bcef5d736121abf799217f8ecae&selectedJob=159943294

try build with adjusted test expectations (pending as of this writing): https://treeherder.mozilla.org/#/jobs?repo=try&revision=03e86a0f36ff5e7ecd366f10e22b4a0cb3bc837c
Comment on attachment 8947611 [details]
Bug 1362115 - don't emit a blob-image for contentless videos.

https://reviewboard.mozilla.org/r/217300/#review223092
Attachment #8947611 - Flags: review?(bugmail) → review+
Comment on attachment 8947613 [details]
Bug 1362115 - adjust reftest expectations for blob-images.

https://reviewboard.mozilla.org/r/217304/#review223098

Squash this into the previous patch
Attachment #8947613 - Flags: review?(bugmail) → review+
Attachment #8947613 - Attachment is obsolete: true
squashed and edited that new file.

Had a syntax error (deleted a ==) so trying again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a66b076a817672788dc041d070d1180c57032b6b
Comment on attachment 8947612 [details]
Bug 1362115 - turn on blob-images by default with webrender.

https://reviewboard.mozilla.org/r/217302/#review223108
Attachment #8947612 - Flags: review?(bugmail) → review+
Keywords: checkin-needed
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71eb4d997c0d
don't emit a blob-image for contentless videos. r=kats
https://hg.mozilla.org/integration/autoland/rev/761a65991c7f
turn on blob-images by default with webrender. r=kats
Keywords: checkin-needed
New changes: 

* skip-if'ing the problematic test temporarily
* fixed some FFI code that was newly crashing

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0636dc46dbc053f4b5324204accd120099afc37
Flags: needinfo?(a.beingessner)
Comment on attachment 8948447 [details]
Bug 1362115 - properly handle empty slices in FFI bindings.

https://reviewboard.mozilla.org/r/217888/#review223668

Looks like there was a lot of discussion about this code when it was added in bug 1347641 and although using as_ptr was mentioned it looks like it got buried under mountains of other FFI/safety discussion. You may want to read it over to make sure we're not accidentally reverting an intentional change though. I didn't really understand a lot of it.
Attachment #8948447 - Flags: review?(bugmail) → review+
Notes on slice thing: confirmed it's just a bug, my fix is correct.

Notes on new push: it seems crashes in graphics are misattributed to future tests because the crash-test runner doesn't block on rendering. The new skip-if is the correct test (isolated with binary search). The issue is a fairly large mask being animated, so that if the frame occurs at some times it is skewed massively (xscale of 57).

This is sufficiently degenerate that we can proceed while skipping the test. I have filed https://bugzilla.mozilla.org/show_bug.cgi?id=1435896 for it.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=930bb57dd24b42b6692e72b728f3844b49f8e588
finally looks good 
Keywords: checkin-needed
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/87b04696a381
don't emit a blob-image for contentless videos. r=kats
https://hg.mozilla.org/integration/autoland/rev/cae458b810a8
properly handle empty slices in FFI bindings. r=kats
https://hg.mozilla.org/integration/autoland/rev/b2804c6bec4c
turn on blob-images by default with webrender. r=kats
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.