Places maintenance causing excessive battery usage

VERIFIED FIXED in Firefox 14

Status

()

Firefox for Android
General
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: Matthieu Pupat, Assigned: gcp)

Tracking

14 Branch
Firefox 16
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox14- verified, firefox15 fixed, blocking-fennec1.0 betaN+, fennec15+)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0) Gecko/20100101 Firefox/12.0
Build ID: 20120420145725

Steps to reproduce:

Browse to www.google.com and leave my phone alone


Actual results:

When I came back Firefox Beta 14 was using 130+M of memory anf 70+% of CPU

Settings > Battery indicates that Firefox is responsible for 21% of CPU usage (2nd position)


Expected results:

Firefox should not use has much CPU and battery
(Reporter)

Updated

5 years ago
OS: Windows 7 → Android
Hardware: x86_64 → ARM
Did you setup Firefox Sync ?
Any other webpages loaded?
How long were you "away" from Firefox?
(Reporter)

Comment 2

5 years ago
Sync is setup.
That was the only loaded page at this time.
I was away for maybe 15 minutes or so. I did not use the phone during that time.
I don't see this with a Galaxy SII. Sync was not enabled. Memory usage was around 50 MB and CPU usage was always below 5%.
(Reporter)

Comment 4

5 years ago
I do not know if this is relevant but I have Galaxy S II with Ice Cream Sandwich. I have seen this battery and CPU usage problem with Firefox 10.0 as well on Ice Cream Sandwich but never seen it on Gingerbread with Firefox 10. I did not try Firefox 14 on Gingerbread.

It does not do it all the time though. I do not know if there is a special pattern that triggers it. When it does not happen memory is abut 30MB and CPU usage is 1 or 2 %.

Is there some logfiles or special things I could do to give you more information? I really like FF14 but this battery thing is really annoying.
If you are able to reproduce it, getting the logcat output from when the CPU usage is high would be useful. See https://wiki.mozilla.org/Mobile/Fennec/Android#Using_logcat for how to get logcat output - there's an app you can use if you don't have the Android SDK installed.

Even better would be if we could reproduce it so that we could attach gdb and see exactly what it's doing, but hopefully the logcat will shed some light on it.
(Reporter)

Comment 6

5 years ago
This morning my battery went under 50% in about 2 hours. Reported battery usage was Firefox 73%, screen 16%. Attached is a log of when Firefox had high CPU usage.
(Reporter)

Comment 7

5 years ago
Created attachment 627144 [details]
Log from when Firefox has high battery usage
CCing gcp, looks like infinite loop of migration errors?
(Assignee)

Comment 9

5 years ago
There's nothing from migration in this log.
Oh, sorry. Do you know what all those PlacesDBUtils errors are, or why they would be in the log?
(Assignee)

Comment 11

5 years ago
If I would I'd have taken the bug :)

The question is what is trying to use Places. We don't have Places in Native Fennec, it's compiled out.
E/GeckoConsole( 1919): [JavaScript Error: "PlacesUtils.bookmarks is undefined" {file: "resource://gre/modules/PlacesDBUtils.jsm" line: 985}]
E/GeckoConsole( 1919): [JavaScript Error: "DBConn is not defined" {file: "resource://gre/modules/PlacesDBUtils.jsm" line: 247}]


This might seem like a silly question, but what version does "about:firefox" say you are running? We should not be using "PlacesDBUtils.jsm" in the latest version of Firefox.
(Reporter)

Comment 13

5 years ago
User agent is

Mozilla/5.0 (Amdroid; Mobile; rv:14.0) Gecko/14.0 Firefox/14.0

Screenshot of about:home attached

I got it from Google Play > Firefox Beta: http://play.google.com/store/apps/details?id=org.mozilla.firefox_beta
(Reporter)

Comment 14

5 years ago
Created attachment 627199 [details]
about:firefox screenshot
Looks like the JS errors are related to Telemetry:
http://mxr.mozilla.org/mozilla-aurora/source/toolkit/components/places/PlacesDBUtils.jsm#985
(Assignee)

Comment 16

5 years ago
The other is in checkIntegrity, is called from "maintenanceOnIdle" and " checkAndFixDatabase".

Perhaps it's called from here:
http://mxr.mozilla.org/mozilla-aurora/source/toolkit/components/places/PlacesCategoriesStarter.js#112

So this looks to me like we could inadvertently have left in some idle maintenance task, which fails because Places isn't really there?

The question for me is why those places JavaScript files are in there at all. Shouldn't we just axe them?
(Assignee)

Comment 17

5 years ago
Created attachment 629192 [details] [diff] [review]
Patch 1. Prevent places javascript from being bundled

Just axe the remaining places parts. This should prevent that idle-maintenance from being called and going bonkers because half of Places is missing anyway.
Assignee: nobody → gpascutto
Attachment #629192 - Flags: review?(mark.finkle)

Updated

5 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Updated

5 years ago
blocking-fennec1.0: --- → ?
status-firefox14: --- → affected
tracking-firefox14: --- → ?
tracking-fennec: --- → 15+
blocking-fennec1.0: ? → soft
blocking-fennec1.0 was already set to soft, so minusing for tracking. With Firefox 15, I expect we'll move back to tracking Fennec issues using the tracking flags.
tracking-firefox14: ? → -
Comment on attachment 629192 [details] [diff] [review]
Patch 1. Prevent places javascript from being bundled

I noticed that we also bundle some of these files in package manifest:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/installer/package-manifest.in#373

We should avoid that too
Attachment #629192 - Flags: review?(mark.finkle) → review-
(Assignee)

Comment 20

5 years ago
Created attachment 629779 [details] [diff] [review]
Patch 1. v2 Prevent places Javascript from being bundled

This axes some more.
Attachment #629779 - Flags: review?(mark.finkle)
(Assignee)

Updated

5 years ago
Attachment #629192 - Attachment is obsolete: true
(Reporter)

Comment 21

5 years ago
Not sure if this is related but Firefox Beta also has a lot of Background Data Usage
(Assignee)

Comment 22

5 years ago
Completely unrelated.
Comment on attachment 629779 [details] [diff] [review]
Patch 1. v2 Prevent places Javascript from being bundled

This looks OK to me. We need to get Marco to review too.
Attachment #629779 - Flags: review?(mark.finkle)
Attachment #629779 - Flags: review?(mak77)
Attachment #629779 - Flags: review+
I suggested time ago to undef MOZ_PLACES to really stop compiling places... now we are adding a further ifdef around everything... sigh.
I'll review shortly, but please file a bug to throw away MOZ_ANDROID_HISTORY and properly use ifdef MOZ_PLACES (that requires doing properly the history implementation instead of the current internal module redirect of History.h).
(Assignee)

Comment 25

5 years ago
Filed bug 761588 for fixing building with MOZ_PLACES=
Comment on attachment 629779 [details] [diff] [review]
Patch 1. v2 Prevent places Javascript from being bundled

Review of attachment 629779 [details] [diff] [review]:
-----------------------------------------------------------------

Need some answer, on the browser part mostly, and some missing removals.

::: browser/components/places/src/Makefile.in
@@ +18,5 @@
>  
>  EXTRA_JS_MODULES = \
>    PlacesUIUtils.jsm \
>    $(NULL)
> +endif

I don't think you use (and probably neither build) any part of browser/, why do you need to exclude these?
Though if you really need to, cause some part of Android browser is shared with the xul browser, you should rather ifdef the places folder in browser/components/Makefile.in

::: mobile/android/installer/package-manifest.in
@@ +372,2 @@
>  @BINPATH@/components/nsLivemarkService.js
>  @BINPATH@/components/nsTaggingService.js

you want to remove these 2 as well.

::: toolkit/components/places/Makefile.in
@@ +101,2 @@
>  
>  TEST_DIRS += tests

you likely want to also exclude tests dir, since you are excluding the manifest they wouldn't work regardless. Not that I think Android is doing anything with these but makes more sense probably.
(Ideally there's much more here you don't need, but that's what the MOZ_PLACES bug should fix)

FWIW, your issue here was PlacesCategoriesStarter.js that is registered in the idle-daily category. Strange that mobile fires idle that often, I was not expecting it to fire at all.
Attachment #629779 - Flags: review?(mak77) → review-
(Assignee)

Comment 27

5 years ago
Created attachment 630489 [details] [diff] [review]
Patch 1. v3 Prevent places javascript from being bundled

Seems like you're right about the browser/ part, that wasn't needed.
Attachment #629779 - Attachment is obsolete: true
Attachment #630489 - Flags: review?(mak77)
Comment on attachment 630489 [details] [diff] [review]
Patch 1. v3 Prevent places javascript from being bundled

Review of attachment 630489 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks for filing the other bug!
Attachment #630489 - Flags: review?(mak77) → review+

Comment 29

5 years ago
(In reply to Matthieu Pupat from comment #21)
> Not sure if this is related but Firefox Beta also has a lot of Background
> Data Usage

Actually seems very related, see bug 762590
Blocks: 762590
(Assignee)

Comment 30

5 years ago
I've created bug 762620 to deal with the idle-daily thing that seems to misfire. Getting rid of the unwanted database stuff can stay in this bug.
(Assignee)

Updated

5 years ago
No longer blocks: 762590
(Assignee)

Updated

5 years ago
Summary: Excessive battery usage → Places maintenance causing excessive battery usage
(Assignee)

Comment 31

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cbd2009a5cd
https://hg.mozilla.org/mozilla-central/rev/6cbd2009a5cd
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
blocking-fennec1.0: soft → betaN+
Comment on attachment 630489 [details] [diff] [review]
Patch 1. v3 Prevent places javascript from being bundled

[Triage Comment]
Please land on mozilla-aurora, mozilla-beta tip, and mozilla-beta MOBILE140_2012061216_RELBRANCH.
Attachment #630489 - Flags: approval-mozilla-beta+
Attachment #630489 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 34

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/81d428506e52
https://hg.mozilla.org/releases/mozilla-beta/rev/0391eb4449c0
(Assignee)

Comment 35

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/18b27d7bcf7a

Updated

5 years ago
status-firefox14: affected → fixed
status-firefox15: --- → fixed
I cannot reproduce this issue on the latest Beta 7 build. The power consumption by Fennec is decent now (about 7% in  15 minutes). Closing bug as verified fixed on:

Device: Galaxy Nexus
OS: Android 4.0.2
Status: RESOLVED → VERIFIED
status-firefox14: fixed → verified
You need to log in before you can comment on or make changes to this bug.