Closed Bug 567339 Opened 11 years ago Closed 11 years ago
Excessive wake-ups after visiting start page
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:188.8.131.52) Gecko/2009060309 Ubuntu/8.04 (hardy) Firefox/3.0.11 Build Identifier: Fennec 2.0a1pre 20100521021132 When Fennec is started it opens about:home which requires https connection for checking available extensions. This causes Fennec to do 5-13 wake-ups/s even if moved to about:blank next. One of the threads seem to keep finding or configuring the missing https connection forever even though connection is not needed anymore after moving to about:blank. Problem seems to be the same if just https proxy is not defined or wrong. Reproducible: Always Steps to Reproduce: 1. Start Fennec 2. Open about:blank 3. Wait couple of minutes 4. Measure with powertop tool Actual Results: Scenarios: A: HTTPS OK -> Start Fennec (about:home) -> open about:blank -> 2.4 wake-ups/s B: HTTPS not OK -> Start Fennec (about:home) -> open about:blank -> 12.0 wake-ups/s Expected Results: Target is to reduce unnecessary wake-ups. Availability of HTTPS connection shouldn't cause neverending extra wake-ups. Eventually number of wake-ups should be the same as in scenario A 2.4 wake-ups. Usually scenario A results in wake-up about between 1-3/s whereas scenario B results between 5-13/s.
Scenario A (HTTPS OK) $ powertop -d -p Top causes for wakeups: 18.0% ( 4.2) [ ] [ata_piix] <interrupt> 15.1% ( 3.5) [ 0] [kernel scheduler] Load balancing tick 11.1% ( 2.6) [ 0] [kernel core] hrtimer_start (tick_sched_timer) 9.1% ( 2.1) [ 1397] gnome-terminal 8.9% ( 2.1) [ ] fennec 8.6% ( 2.0) [ 0] [kernel core] clocksource_watchdog (clocksource_watchdog) Scenario B (HTTPS not OK) (5 minutes after going to about:blank) $ powertop -d -p Top causes for wakeups: 22.2% ( 6.1) [ ] fennec 14.1% ( 3.9) [ 0] [kernel scheduler] Load balancing tick 13.7% ( 3.7) [ ] [ata_piix] <interrupt> 9.5% ( 2.6) [ 0] [kernel core] hrtimer_start (tick_sched_timer) 7.6% ( 2.1) [ 1397] gnome-terminal 7.3% ( 2.0) [ 0] [kernel core] clocksource_watchdog (clocksource_watchdog)
Marko: I am investigating this issue, and can confirm it. However, it doesn't seem to be related to HTTPS, but to something else - imgContainer::Notify is called 4 times/second, which accounts for the 4 additional wakeups between the two cases shown in your last comment. HTTPS is indeed used (downloading CRLs, etc.), but that activity appears to stop eventually. Perhaps there are two issues here? What led you to suspect HTTPS as the problem?
Alon: The reason I suspect HTTPS is that if I have set the HTTPS/SSL proxy correctly, I get 2-3 wakeups/s. If HTTPS proxy isn't set or internet connection is not working, I get 5-13 wakeups/s. Also on start page about:home Fennec is starting to check up over HTTPS if there are extension updates available. If browser is started with -url about:blank although proxy is not configured, we are still between 2-3 wakeups/s. Of course it's possible that this is only a way to trigger another problem, for example calling of imgContainer::Notify function 4 times/second.
Marko: Thanks for the info, that is the same data as I am seeing. Then it turns out that indeed this is just triggering another problem: Depending on HTTPS working or not, a progress animation is shown, and that animation starts a timer that persists even after it is needed. I am investigating now how to resolve this.
I marked this as depending on bug 359608, however, I am not certain anymore. That bug report's comments imply that it's fixed by disabling bfcache (max_total_viewers), however, that does *not* fix this bug here. So there isn't a workaround for this bug.
This sounds like it might just be a symptom of bug 502694? Are you sure the timers are all from the animated image, and not just everything else?
We should really just stop using self-timers for animated images and switch them to refresh driver.... :(
(In reply to comment #6) > This sounds like it might just be a symptom of bug 502694? Are you sure the > timers are all from the animated image, and not just everything else? I don't think this is the same as that bug. I am seeing imgContainer::Notify being called at the rate of the animation, and not imgContainer::sDiscardTimerCallback (which should anyhow be called only every 15 seconds). It looks like the animation timer is only halted when the imgContainer is destroyed (it then destroys mAnim, which cancels the timer). And if I am reading the stack traces I gathered correctly, the imgContainer is only destroyed when the HTML image element is destroyed, which depends on garbage collection.
> It looks like the animation timer is only halted when the imgContainer is > destroyed That's not quite true; see the StopAnimation call in imgRequest::RemoveProxy. > when the HTML image element is destroyed, which depends on garbage collection. This part is true, though, since the proxy is owned by the <img>. There was talk at some point about differentiation which proxies care about animation (the one owned by <img> does not, really).
I tried this issue by just opening an animated gif or png from local file system and noticed that when you move to about:blank after that there will be the same frequency of wake-ups that is seen with animated picture. This doesn't happen if you do the same thing with a web page that has animated pictures, so there is some difference on how timers behave when comparing local and web handling.
Do we need a separate Core:Imagelib bug covering that issue?
(In reply to comment #10) > I tried this issue by just opening an animated gif or png from local file > system and noticed that when you move to about:blank after that there will be > the same frequency of wake-ups that is seen with animated picture. This doesn't > happen if you do the same thing with a web page that has animated pictures, so > there is some difference on how timers behave when comparing local and web > handling. This is different from what I am seeing. I see the same wakeups regardless of whether the image is from the web or is local content. How are you measuring wakeups? (Over here I am measuring wakeups by printed output from the imgContainer animation callback, and also using powertop).
(In reply to comment #12) > This is different from what I am seeing. I see the same wakeups regardless of > whether the image is from the web or is local content. How are you measuring > wakeups? (Over here I am measuring wakeups by printed output from the > imgContainer animation callback, and also using powertop). I noticed what I reported using http://www.gifs.net. Now I tested it again and going to about:blank was working. I guess it is easiest to notice when selecting some category and viewing many animated GIFs at the same time, but it can also be noticed by viewing a preview of a single picture. I'm using powertop for measurements. Since you didn't get the same result, I also tested this with my own web page with only text and one picture. Going to about:blank didn't work with that one, so now I can only say that going to about:blank will stop most of the timers after viewing animated pictures in from http://www.gifs.net. Not much of a test case.
These wake-up issues may have been tolerated in firefox, but with fennec they would need to be resolved. Could the first step be to allow suspending animation for all the imgContainers, by making it an observer? There is a lot of these problems with animation in meta bug 119597 and many, possible duplicates, added later not visible in that bug. With this approach it would be easy to suspend all imgContainers when wanted.
The different results might be related to garbage collection, btw - GC will eventually remove the unnecessary timers. So that could explain seeing somewhat different results at different times / different systems. About the patch, I am not sure when we would want to suspend all the animations?
(In reply to comment #15) > The different results might be related to garbage collection, btw - GC will > eventually remove the unnecessary timers. So that could explain seeing somewhat > different results at different times / different systems. > > About the patch, I am not sure when we would want to suspend all the > animations? Suspending all animations are needed for example, when mobile device is idle. Especially if system goes to sleep, all this kind of unnecessary things need to be suspended. On that note there is also a general problem with nsIIdleService. It wont start counting the idle time before user has been active. This leads to situation where idle timer wont start if you only start the fennec and user wont do anything after that. This was noticed when doing patch for this that would use nsIIdleService for suspending the timers after some period of time.
Makes sense, I would prefer though to actually freeze the entire child process when the mobile device is idle (bug 568054) instead of writing code to manually suspend every system separately. But both approaches have advantages over the other. As the problem with the idle service is a separate issue, maybe it makes sense to file a separate bug.
I posted a patch to bug 359608, which according to my tests also fixes this bug, https://bugzilla.mozilla.org/show_bug.cgi?id=359608#c16 See details there about how it relates to the refresh driver.
(In reply to comment #16) > Suspending all animations are needed for example, when mobile device is idle. > Especially if system goes to sleep, all this kind of unnecessary things need to > be suspended. Regarding that, btw, it seems mobile OSes may (eventually, or already?) do something like it, http://lwn.net/Articles/390369/ OS-level suspending of all applications (even running ones) would be better than anything we can do, probably.
It is starting to look like the patch for bug 359608 (which we depend on) will fix everything *but* chrome pages... and the Fennec start page is chrome. So we may need a different solution here.
Assignee: nobody → azakai
Jon is this fix still needed? Alon any progress here
Comment on attachment 450633 [details] [diff] [review] Patch that would enable suspending the animation At least this patch is obsolete and not approach that is wanted. Alon were you planning on some fix for this? I guess based on previous comments this still needs one?
Attachment #450633 - Attachment is obsolete: true
We definitely need a fix for this. Some thoughts, about discussion with bholley and dbaron: 1. aboutHome.xhtml, the page for this, just hides the image document.getElementById("loadingAddons").style.display = "none"; Perhaps we can fix this by removing the element, not just hiding it. 2. We can in theory let hidden images be properly frozen, but we'd need to fast-forward them when shown again - so we need the fast-forward code that will be written for suspending animation in background tabs, bug 588975. It would not be trivial to get this to work with propagation though - in the sense of freezing an image in an element, and that element is hidden (but not directly the image). 3. We need to make sure that mIsActive is set properly in the Fennec browser code; Firefox's browser code does this. It is necessary for freezing animations in the bfcache. 4. To get this page to work as is, we would need to add code so that chrome stuff handles animation freezing etc. - not sure if it would or wouldn't (it is html, not xul - but it's chrome). 5. Hacks/workarounds: We can make the image animate once instead of repeatedly. If the animation is 5 seconds long, would users ever see that much of it anyhow?
(In reply to comment #23) > We definitely need a fix for this. Some thoughts, about discussion with bholley > and dbaron: > > 1. aboutHome.xhtml, the page for this, just hides the image > > document.getElementById("loadingAddons").style.display = "none"; > > Perhaps we can fix this by removing the element, not just hiding it. File a bug and I'll make a patch > 3. We need to make sure that mIsActive is set properly in the Fennec > browser code; Firefox's browser code does this. It is necessary for > freezing animations in the bfcache. What is mIsActive? We do set nsIDocShell.isActive for content pages here: http://mxr.mozilla.org/mobile-browser/source/chrome/content/content.js#462 and 468 but we don't for chrome pages! File a bug?
Filed those two bugs, added as blockers for this bug.
With the resolving of those two bugs, this bug is resolved as well. The startup page no longer kills our battery.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Verified fixed on: Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110922 Firefox/9.0a1 Fennec/9.0a1 Device: Samsung Galaxy S OS: Android 2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.