1254151 - Higher CPU usage (Firefox and system overall) when playing YouTube video

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Marco Castelluccio [:marco]]

CPU usage of the content process is almost the same, the parent process uses a lot more CPU.

The overall system's CPU usage is a lot higher, as you can see from the screenshots that I'm going to attach.

mozregression points me to bug 1241832.

Comment 1

[Marco Castelluccio [:marco]]

Attachment: before.png

Comment 2

[Marco Castelluccio [:marco]]

Attachment: after.png

Comment 3

[Lee Salzman [:lsalzman]]

Are you using the flash-based video player or html5?

Comment 4

[Marco Castelluccio [:marco]]

HTML5.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Can you copy & paste the contents of "Stats for nerds" from the context menu on the bug?

Comment 6

[Marco Castelluccio [:marco]]

Video ID: EIzVgmdcq_s
Dimensions: 854 x 480 * 2
Resolution: 1280 x 720@56
Volume: 100%
Stream Host: r16---sn-5hnednes
Stream Type: https
CPN: B410aeA1HPGpzExG
Mime Type: video/webm; codecs="vp9"
DASH: yes (302/250)
Connection Speed: 2270 Kbps
Dropped Frames: 0/444

Comment 7

[Lee Salzman [:lsalzman]]

Attachment: cairo-shm-format-fast-path.diff

Comment 8

[Lee Salzman [:lsalzman]]

Attachment: use B8G8R8X8 for 24 bit depth visuals in nsShmImage with Cairo

We're missing a fast-path in pixman for upscaling because pixman does not implement fast-paths for bilinear filtering of BGRX onto BGRA. It only implements BGRA->BGRA, BGRX->BGRX, and BGRA->BGRX.

Some Skia changes we made had nsShmImage report its format as BGRA to keep Skia happy, since Skia does not really actually support opaque destination formats, only source formats. That seems to have bitten us a bit here for bilinear filtering.

So, the patch checks if we're using Cairo, and if so uses BGRX, otherwise goes with BGRA. Verified in the profiler we're back on the bilinear fast-path in pixman, which should help the CPU usage a bit, though it's never going to quite look as good as just using the GPU.

Comment 9

[Lee Salzman [:lsalzman]]

Try run for Cairo nsShmImage fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f98c751f7f73

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/12cdf126c9a4

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/12cdf126c9a4

Comment 12

[Lee Salzman [:lsalzman]]

Comment on attachment 8727708 [details] [diff] [review]
use B8G8R8X8 for 24 bit depth visuals in nsShmImage with Cairo

Approval Request Comment
[Feature/regressing bug #]: 1239152 (Feb 19, 2016)
[User impact if declined]: Significantly decreased fullscreen video performance (i.e. Youtube) 
[Describe test coverage new/current, TreeHerder]: mochitest, reftest
[Risks and why]: Basically none. This patch just restores prior behavior when using the Cairo backend.
[String/UUID change made/needed]: None

Comment 13

[Marco Castelluccio [:marco]]

(In reply to Lee Salzman [:lsalzman] from comment #8)
> So, the patch checks if we're using Cairo, and if so uses BGRX, otherwise
> goes with BGRA. Verified in the profiler we're back on the bilinear
> fast-path in pixman, which should help the CPU usage a bit, though it's
> never going to quite look as good as just using the GPU.

Thanks, this is much better, almost as before bug 1241832.

(In reply to Lee Salzman [:lsalzman] from comment #12)
> [User impact if declined]: Significantly decreased fullscreen video
> performance (i.e. Youtube) 

Also non-fullscreen :)

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8727708 [details] [diff] [review]
use B8G8R8X8 for 24 bit depth visuals in nsShmImage with Cairo

Perf improvement, taking it in Aurora47.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8a2c0e4cb868