Closed Bug 1254151 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox46 --- unaffected
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: marco, Assigned: lsalzman)

References

(Blocks 1 open bug, )

Details

(Keywords: perf)

Attachments

(3 files, 1 obsolete file)

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.
Attached image before.png
Attached image after.png
Are you using the flash-based video player or html5?
Flags: needinfo?(mcastelluccio)
HTML5.
Flags: needinfo?(mcastelluccio)
Can you copy & paste the contents of "Stats for nerds" from the context menu on the bug?
Flags: needinfo?(mcastelluccio)
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
Flags: needinfo?(mcastelluccio)
Attached patch cairo-shm-format-fast-path.diff (obsolete) — Splinter Review
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.
Assignee: nobody → lsalzman
Attachment #8727707 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8727708 - Flags: review?(jmuizelaar)
Attachment #8727708 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/12cdf126c9a4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
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
Attachment #8727708 - Flags: approval-mozilla-aurora?
(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 on attachment 8727708 [details] [diff] [review]
use B8G8R8X8 for 24 bit depth visuals in nsShmImage with Cairo

Perf improvement, taking it in Aurora47.
Attachment #8727708 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
No longer depends on: unaccel-video
No longer blocks: 1263222
You need to log in before you can comment on or make changes to this bug.