Closed Bug 985369 Opened 7 years ago Closed 7 years ago

Writing into the URL bar consumes 100% CPU time

Categories

(Firefox OS Graveyard :: Gaia::Browser, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(b2g-v1.4 fixed, b2g-v2.0 verified)

VERIFIED FIXED
1.4 S5 (11apr)
Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- verified

People

(Reporter: gsvelto, Assigned: gsvelto)

References

Details

(Keywords: perf, power, Whiteboard: [c=power p= s= u=])

Attachments

(3 files)

+++ This bug was initially created as a clone of Bug #980070 +++

While investigating the behavior described in bug 980070 comment 36 I've noticed that when we type some characters in the URL bar the suggestions/history elements appearing below are refreshed for every character. This drives CPU utilization to 100% and when typing fast causes the updates to pile up and be redrawn with a measurable delay.

Could we batch up those updates so that if the user is typing too fast for us to keep up we skip the completions we can't come up with fast enough?

Quickly looking at the code in apps/browser/js/awesomescreen.js I think this might be achieved by canceling the call to AwesomeScreen.populateResults() if AwesomeScreen.update() is invoked again before the former has had time to repopulate the list of results.
See Also: → 980070
leaving qawanted here to see if this problem is present on 1.1
Keywords: qawanted
Priority: -- → P1
On Buri v1.1 I see the CPU float around 91-94% during writing into the URL bar.

Prerequisites:
use 'adb shell top -m 10' to observe CPU usage

STRs
1. Launch Browser App
2. Activate URL bar
3. Begin typing quickly and observe CPU consumption

Actual Results:
91-94%

v1.1 Environmental Variables:
Device: Buri v1.1 MOZ
BuildID: 20140318041202
Gaia: 44a2ddf63373f8e95c784faf4ed4d60081699c61
Gecko: 2c70ef07c5b3
Version: 18.0
Firmware Version: v1.2-device.cfg
Keywords: qawanted
QA Contact: pbylenga
This patch implements what I described in comment 0. Basically every time an update to the suggestions is triggered we check if another one is already undergoing. If it is, we flag the new update as pending and wait for the previous one to finish before executing it. Since at all times we can have only one pending update, updates arriving rapidly one after the other are automatically skipped reducing the CPU load.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Attachment #8397915 - Flags: review?(bfrancis)
Since I was already working on this and I'm kind-of a linter freak I took the liberty of preparing a patch to make this file jshint-clean too. The changes are minimal.
Attachment #8397916 - Flags: review?(bfrancis)
And here comes the pull request with both patches included
Comment on attachment 8397915 [details] [diff] [review]
[PATCH 1/2] Drop updates to the suggestions that have already been superseded

That's great, thanks :)
Attachment #8397915 - Flags: review?(bfrancis) → review+
Attachment #8397916 - Flags: review?(bfrancis) → review+
Merged into master https://github.com/mozilla-b2g/gaia/commit/71909da8662ed3b600a04dd70c91aa567566cd55
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Oops - looks like this cross paths with another merge before it landed and broke the linter. Follow-up landed here: https://github.com/mozilla-b2g/gaia/commit/b6ba925df0afa3a1d5d5a197457b9ce51abe5fc1
Ah, thanks Kevin!
The bug this one was cloned from was a 1.3 blocker so since we already have a fix here and it's pretty straightforward let's nom this for 1.4.
blocking-b2g: --- → 1.4?
(In reply to Gabriele Svelto [:gsvelto] from comment #11)
> The bug this one was cloned from was a 1.3 blocker so since we already have
> a fix here and it's pretty straightforward let's nom this for 1.4.

That sounds like a case that we should actually ask for approval, rather than block. This is mainly because this has already been present on 1.1 & 1.3, so we've already shipped with this problem previously. So I'd suggest asking for 1.4 approval.
blocking-b2g: 1.4? → ---
Comment on attachment 8397915 [details] [diff] [review]
[PATCH 1/2] Drop updates to the suggestions that have already been superseded

(In reply to Jason Smith [:jsmith] from comment #12)
> That sounds like a case that we should actually ask for approval, rather
> than block. This is mainly because this has already been present on 1.1 &
> 1.3, so we've already shipped with this problem previously. So I'd suggest
> asking for 1.4 approval.

Good point, here's the request:

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): this has always been the default behavior
[User impact] if declined: higher CPU consumption when typing in the URL bar, no functional issues
[Testing completed]: tested on the emulator and on a device, landed on master and stuck (until now at least)
[Risk to taking this patch] (and alternatives if risky): there might be an impact on the suggestions presented when typing an URL
[String changes made]: none
Attachment #8397915 - Flags: approval-gaia-v1.4?
(In reply to Gabriele Svelto [:gsvelto] from comment #13)
> Comment on attachment 8397915 [details] [diff] [review]
> [PATCH 1/2] Drop updates to the suggestions that have already been superseded
> 
> (In reply to Jason Smith [:jsmith] from comment #12)
> > That sounds like a case that we should actually ask for approval, rather
> > than block. This is mainly because this has already been present on 1.1 &
> > 1.3, so we've already shipped with this problem previously. So I'd suggest
> > asking for 1.4 approval.
> 
> Good point, here's the request:
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): this has always been the default
> behavior
> [User impact] if declined: higher CPU consumption when typing in the URL
> bar, no functional issues
> [Testing completed]: tested on the emulator and on a device, landed on
> master and stuck (until now at least)
> [Risk to taking this patch] (and alternatives if risky): there might be an
> impact on the suggestions presented when typing an URL
Could you elaborate on this ? I am requesting QA to verify this patch before uplifting due to the outlined risk or get some exploratory testing here and may be any input on testcases might be helpful to them here.
Flags: needinfo?(gsvelto)
Keywords: verifyme
(In reply to bhavana bajaj [:bajaj] from comment #14)
> Could you elaborate on this ? I am requesting QA to verify this patch before
> uplifting due to the outlined risk or get some exploratory testing here and
> may be any input on testcases might be helpful to them here.

One of the earlier versions of my patch had an issue which caused some suggestions not to show up. I noticed it because the text typed in the URL and the one looked up in the suggestions did not match so I fixed it. I still wanted to mention this because I'm fairly confident that the current patch doesn't have that problem but it's always better to be safe than sorry with testing.
Flags: needinfo?(gsvelto)
Leaving a NI on Peter to see if he can help verify this bug on master/1.5.
Flags: needinfo?(pbylenga)
On today's Master/1.5 when I type in the URL usage floats between 62-74% using steps outlined in Comment 2.

v1.5 Environmental Variables:
Device: Buri v1.5 MOZ
BuildID: 20140408040204
Gaia: 1958454595b1fa0e061f0652ae965629993f5708
Gecko: 8883360b1edb
Version: 31.0a1
Firmware Version: v1.2-device.cfg
Status: RESOLVED → VERIFIED
Flags: needinfo?(pbylenga)
Keywords: verifyme
Attachment #8397915 - Flags: approval-gaia-v1.4? → approval-gaia-v1.4+
Cherry-picked to gaia/v1.4 6c4319c95114dc4bd18b2497306b6cbac3b29ad9

https://github.com/mozilla-b2g/gaia/commit/6c4319c95114dc4bd18b2497306b6cbac3b29ad9
Target Milestone: --- → 1.4 S5 (11apr)
You need to log in before you can comment on or make changes to this bug.