856602 - CPU usage when viewing pages that contains mjpeg videos

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Josep Sanz]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:22.0) Gecko/20130401 Firefox/22.0
Build ID: 20130401030817

Steps to reproduce:

I try to see this page some times:
- http://www.oadm.cat/en/content/61/live-webcams.htm


Actual results:

And when go to another page, the CPU usage is 100%!!!


Expected results:

I can understand that while the mjpeg video is running, firefox can require more amount of CPU but when I close or I go to another page, the cpu usage must be normal.

Comment 1

[Alice0775 White]

I can reproduce
http://hg.mozilla.org/mozilla-central/rev/0b7c27024048
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130401 Firefox/22.0 ID:20130401030817

Comment 2

[Alice0775 White]

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/bef19bca23f9
Mozilla/5.0 (Windows NT 5.1; rv:22.0) Gecko/20130325 Firefox/22.0 ID:20130325015122
Bad:
http://hg.mozilla.org/mozilla-central/rev/4d3250f3afea
Mozilla/5.0 (Windows NT 5.1; rv:22.0) Gecko/20130325 Firefox/22.0 ID:20130325093524
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bef19bca23f9&tochange=4d3250f3afea


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/71fe5b69c834
Mozilla/5.0 (Windows NT 5.1; rv:22.0) Gecko/20130324 Firefox/22.0 ID:20130324205822
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/1541b7a03e17
Mozilla/5.0 (Windows NT 5.1; rv:22.0) Gecko/20130324 Firefox/22.0 ID:20130324220925
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=71fe5b69c834&tochange=1541b7a03e17

Comment 3

[Alice0775 White]

I can also reproduce on http://www.shioda-dcom.co.jp/shiocame/

Comment 4

[Alex Keybl [:akeybl]]

May need to discuss with other contributors to bug 716140 about ownership here.

Comment 5

[Robert Lickenbrock [:rclick]]

I cannot reproduce this with my patches in Bug 854803, but can without.

I'm going to optimistically call this a dupe. Please reopen if there is reason to believe otherwise.

Comment 6

[Alice0775 White]

CPU usage is low during play the mjpeg on the try build.

However, I can still reproduce the High CPU usage after close the tab.

http://hg.mozilla.org/try/rev/8360f6cd74a9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130331 Firefox/22.0 ID:20130331231006

Comment 7

[Alice0775 White]

The try server build of Bug 856486 Comment#6 does not fix the high CPU usage after close the tab :(

http://hg.mozilla.org/try/rev/6a78b2e8ecb9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130402 Firefox/23.0 ID:20130402173333

Comment 8

[Alice0775 White]

Stack during the high CPU usage after close the tab.
bp-73703684-cd7c-439b-8ff1-0e8fd2130403
(Stack using http://benjamin.smedbergs.us/crashfirefox.exe)

Comment 9

[Alice0775 White]

The try server build of Bug 856486 Comment#8 does not fix the high CPU usage after close the tab :(

http://hg.mozilla.org/try/rev/9aa6bef16179
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20130403 Firefox/23.0 ID:20130403135600

Comment 10

[Seth Fowler [:seth] [:s2h]]

I can confirm this issue occurs on OS X as well, even with all of the various mJPEG-related fixes that are currently posted applied. Investigating now.

Comment 11

[Seth Fowler [:seth] [:s2h]]

Attachment: RasterImage::DecodeJob should not reschedule itself if it can't make progress.

The problem seems to be that the image decoder thread keeps spinning away even when you leave the page, although it cannot make any more progress (it's not getting more data delivered to it), because we don't have a termination condition for it that covers this case. IMO the original termination conditions should have been sufficient, and the real cause is bug 857876, but that seems _much_ harder to fix, and I have no confidence the fix will come quickly, so I think it's still worth adding a termination condition to resolve this problem right away.

This patch is essentially a one-liner that says "if the DecodeJob didn't make any progress this time, it shouldn't reschedule itself".

Comment 12

[Seth Fowler [:seth] [:s2h]]

Try job here: https://tbpl.mozilla.org/?tree=Try&rev=0349128958e7

Comment 13

[Alice0775 White]

(In reply to Seth Fowler [:seth] from comment #12)
> Try job here: https://tbpl.mozilla.org/?tree=Try&rev=0349128958e7

The try server build works very well and fixed the CPU problem.

http://hg.mozilla.org/try/rev/0349128958e7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130403 Firefox/23.0 ID:20130403190924
http://hg.mozilla.org/try/rev/0349128958e7
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20130403 Firefox/23.0 ID:20130403191148

Comment 14

[Seth Fowler [:seth] [:s2h]]

Thanks for the verification, Alice. The try results look good too.

Comment 15

[Timothy Nikkel (:tnikkel)]

Comment on attachment 733153 [details] [diff] [review]
RasterImage::DecodeJob should not reschedule itself if it can't make progress.

This seems reasonable to me.

Comment 16

[Seth Fowler [:seth] [:s2h]]

Thanks for the review!

Pushed to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/8161280e740e

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8161280e740e

Comment 18

[Joe Drew (not getting mail)]

We should get this into 22.

Comment 19

[Seth Fowler [:seth] [:s2h]]

Sure. There are a couple of other related bugs. I will request uplift for all of them.

Comment 20

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 733153 [details] [diff] [review]
RasterImage::DecodeJob should not reschedule itself if it can't make progress.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 716140
User impact if declined: Massive CPU usage regression. Will persist for the entire session once the user views an mJPEG image, even if they leave the page.
Testing completed (on m-c, etc.): On m-c without problems since 2013-04-05.
Risk to taking this patch (and alternatives if risky): Low. This is a very small patch that prevents a DecodeJob from continuing if it isn't making progress. Even if this were to happen for reasons unrelated to this bug, a new DecodeJob would still be scheduled if more data eventually did arrive, so this should be a safe choice.
String or IDL/UUID changes made by this patch: None.

Comment 21

[Seth Fowler [:seth] [:s2h]]

Thanks Lukas!

https://hg.mozilla.org/releases/mozilla-aurora/rev/3193c9695b2e
https://hg.mozilla.org/releases/mozilla-aurora/rev/88773e6debe3

Comment 22

[Mihai Morar, (:MihaiMorar)]

I confirm the fix is verified on FF 23b6: Build ID: 20130715155216