Last Comment Bug 777266 - Official "Search by Image (by Google)" v1.0.6 add-on causes high CPU utilization
: Official "Search by Image (by Google)" v1.0.6 add-on causes high CPU utilization
Status: RESOLVED FIXED
[Snappy][sps][3rd-party-bustage]
:
Product: Tech Evangelism
Classification: Other
Component: Add-ons (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal with 4 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on: 781269
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-25 02:01 PDT by Frank Freibuth
Modified: 2013-12-27 14:24 PST (History)
24 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
-
-


Attachments

Description Frank Freibuth 2012-07-25 02:01:50 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0
Build ID: 20120724042008

Steps to reproduce:

The official "Search by Image (by Google) v1.0.6" extension as linked from Google's official website:
http://www.google.com/insidesearch/features/images/searchbyimage.html ->
https://addons.mozilla.org/en-US/firefox/addon/search-by-image-by-google/
causes a massive CPU in current Firefox builds (15/16, possibly earlier). The same extension worked fine in earlier Firefox builds.


Actual results:

Install "Search by Image (by Google) v1.0.6" and use Firefox for a while. firefox.exe's CPU usage will slowly creep up over a period of one or two hours and eventually fully and permanently max out one CPU-core until the browser is restarted.
Disabling or uninstalling "Search by Image (by Google) v1.0.6" fixes it. Unofficial add-ons with the same functionality appear to be unaffected.


Expected results:

A simple add-on like this shouldn't cause a severe CPU leak.
Comment 1 RNicoletto 2012-07-25 06:29:16 PDT
Mozillazine forum discussion ==> http://forums.mozillazine.org/viewtopic.php?f=23&t=2506211
Comment 2 Ivan Garcia 2012-07-25 07:02:27 PDT
Happened to me on both XP and Win7. You don't have to actually use the extension's functionality at all for the problem to manifest itself.
Comment 3 Matthias Versen [:Matti] 2012-07-25 14:46:29 PDT
Requesting tracking to get attention to this report.
Comment 4 Andrew McCreight [:mccr8] 2012-07-25 14:48:39 PDT
Does this happen on Nightly, too?  Maybe somebody could profile it using Nightly.

https://developer.mozilla.org/en/Performance/Profiling_with_the_Built-in_Profiler
Comment 5 Alex Thompson 2012-07-25 19:23:13 PDT
I installed the extension in Firefox 17 (2012-07-25) nightly on Linux (x86_64).

Immediately after restarting the browser, fx was using ~50% cpu. After 20-30mins it rose to 90%. (After disabling the extension, CPU is behaving normally with idle use ~5%)

My profiling info is at the link below
http://people.mozilla.com/~bgirard/cleopatra/?report=09c2de8f177726eebc7231bacad381012027c255
Comment 6 Andrew McCreight [:mccr8] 2012-07-25 19:29:02 PDT
Thanks for the profile!  It looks like 42% of the time is being spent running Jquery.  I'm going to throw this over into JS.  Maybe a JS person can make some sense of this.  Odds are it is the addon doing something silly, though.
Comment 7 Jorge Villalobos [:jorgev] 2012-07-25 19:53:31 PDT
We were informed about this problem a few days ago, which is why the add-on was downgraded from fully public to preliminarily reviewed. We have notified the developers about this, but haven't heard from them yet.

The problem with the add-on is that it is registering browser-wide mousemove event listeners, which unnecessarily affects performance all around. Kris filed bug 775716 so that there are validation warnings for this when submitting to AMO.
Comment 8 Frank Freibuth 2012-07-26 02:27:43 PDT
There are two problems:

1. The downgrade to "preliminarily reviewed" makes little difference in user adoption as long as the add-on is still linked to from the official Google Reverse Image Search website http://www.google.com/insidesearch/features/images/searchbyimage.html and apparently, whoever is responsible for the add-on, has just changed the link from pointing to the AMO site to pointing directly to the still broken and unchanged extension at https://dl.google.com/searchbyimage/searchbyimage_latest.xpi (less than 24 hours ago it was pointing to https://addons.mozilla.org/en-US/firefox/addon/search-by-image-by-google/ instead) - so somebody must be monitoring the extension and is now taking all the wrong actions (ensuring unhindered availability of the broken extension instead of pulling or fixing it).

2. Yes, registering browser-wide mousemove event listeners is terrible and completely unnecessary for this (and just about any) extension.
But it used to not have this bad of an effect on firefox.exe - in earlier builds this was barely notable, now firefox.exe eventually permanently maxes out the CPU - this still sounds like a bug in Firefox to me that should be addressed. What's the point on cracking down memory leaks in add-ons if other add-ons will now end up permanently maxing out the CPU instead?
Comment 9 Andrew McCreight [:mccr8] 2012-07-26 06:45:12 PDT
(In reply to Frank Freibuth from comment #8)
> 2. Yes, registering browser-wide mousemove event listeners is terrible and
> completely unnecessary for this (and just about any) extension.
> But it used to not have this bad of an effect on firefox.exe - in earlier
> builds this was barely notable, now firefox.exe eventually permanently maxes
> out the CPU - this still sounds like a bug in Firefox to me that should be
> addressed. What's the point on cracking down memory leaks in add-ons if
> other add-ons will now end up permanently maxing out the CPU instead?

What version is unaffected by this problem?  On the mozillazine thread, some people said 14 was affected and some seemed to be saying it wasn't.  Are we sure that it is a change in the browser and not, say, the addon?

Olli, can you think of anything that might have caused registering browser-wide mousemove event listeners to perform badly starting in 14 or 15?
Comment 10 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-07-26 07:51:26 PDT
(In reply to Andrew McCreight [:mccr8] from comment #9)
> Olli, can you think of anything that might have caused registering
> browser-wide mousemove event listeners to perform badly starting in 14 or 15?
Registering event listeners hasn't changed, nor event handling.
But something that the event listener actually does may have changed.
If the profile in comment 5 is reliable, it is running some JS which takes all the time.

I wonder if it was possible to get a profile using a Nightly. I think the profiler can give better information in latest builds.
Comment 11 Kris Maglione [:kmag] 2012-07-26 08:07:41 PDT
That event listener is seriously expensive. I doubt very much that any changes Firefox-side are at fault: https://gist.github.com/3182602
Comment 12 Kris Maglione [:kmag] 2012-07-26 08:11:54 PDT
Hm. Actually, I guess most of the work it does doesn't happen with the default settings, which puts it down to the high frequency of events and the work done by jQuery during dispatch.
Comment 13 Andrew McCreight [:mccr8] 2012-07-26 08:14:20 PDT
Yeah, the profile above indicated that the largest single chunk of time was being spent in Jquery.
Comment 14 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-07-26 08:38:43 PDT
Anyhow, regression range would be *very* useful.
Comment 15 Igor Strielov 2012-07-26 10:22:14 PDT
For me, this issue manifested itself after Fx upgrade from 13.0.2 to 14.0.1. On Fx 13 this extension did not cause any noticeable CPU usage.
Comment 16 Ivan Garcia 2012-07-26 13:28:06 PDT
I had the same experience as Igor. Issue began with the upgrade to Firefox 14.
Comment 17 (mostly gone) XtC4UaLL [:xtc4uall] 2012-07-26 13:43:39 PDT
For anyone able to repro you can try to find a Range using http://mozilla.github.com/mozregression/ and e.g. using

mozregression --repo=mozilla-inbound --good=2012-04-22 (Nightly got 14 minus few Days)
Comment 18 Alex Keybl [:akeybl] 2012-07-26 15:19:32 PDT
(In reply to Igor Strielov from comment #15)
> For me, this issue manifested itself after Fx upgrade from 13.0.2 to 14.0.1.
> On Fx 13 this extension did not cause any noticeable CPU usage.

If we're able to prove this is a regression from 13->14, this may not end up being a TE bug.
Comment 19 redrock59 2012-07-26 15:49:40 PDT
I experienced this issue on 2 different systems while at 13.0.2. I upgraded to 14.0.1 in hopes it would be resolved. Anecdotal at best, but the issue definitely existed on 13.0.2, at least for me. Disabling the Search by Image add-on quieted my CPU usage for Firefox from 30+% to 1-3%.
Comment 20 Alex Thompson 2012-07-26 19:25:26 PDT
I quickly tested the extension on Fx 14.0.1, 12.0 and 7.0 (Linux x86-64). After opening a number of image heavy pages, closing them and leaving the browser idle for a few minutes. All showed idle CPU usage of 30-40% even when the window wasn't focused. I didn't test longer term usage however, so on the older versions the CPU usage may not climb to ~100%.

This doesn't look like a firefox regression, more a problem with the extension.
Comment 21 Frank Freibuth 2012-07-28 04:21:29 PDT
I'm on the Aurora channel and I only noticed this massive CPU consumption after going from Aurora 14 to 15. That doesn't mean it wasn't there before, but similar reports on the forum seem to suggest that this at least wasn't always this much of a problem.

Either way, considering the severity of the way the bug, the fact that Google's team responsible for the add-on is seemingly impossible to contact by mere mortals and that they actively ensure easy availability of the add-on without fixing it - shouldn't something be done? Would blacklisting be an option, especially considering that there are at least 3 simple add-ons with essentially 100% identical features that aren't affected by this problem?
Comment 22 Ivan Garcia 2012-07-28 09:00:08 PDT
Looks like the Google devs responded to a bunch of bad reviews on the add-on page and said that they're working on it:

https://addons.mozilla.org/en-US/firefox/addon/search-by-image-by-google/reviews/

"Thank you for reporting the problem. We have submitted a fix and it is currently pending review."
Comment 23 Kris Maglione [:kmag] 2012-07-30 11:46:53 PDT
(In reply to Frank Freibuth from comment #21)
> shouldn't something be done? Would blacklisting be an option,
> especially considering that there are at least 3 simple add-ons
> with essentially 100% identical features that aren't affected by
> this problem?

We already downgraded the add-on to preliminary review, from its
previous full review status. We generally do not blocklist for
issues like this unless they affect large numbers of users.
Comment 24 Frank Freibuth 2012-07-30 14:12:38 PDT
Well, it does affect everyone using the plugin and considering that it's an official Google plugin, that should be quite a few users.

The devs claimed they fixed it in v1.0.7 but CPU load is still enormous with the updated 1.0.7 in Aurora. Now they think they fixed it but clearly have not - that's not a good sign.
Comment 25 Kris Maglione [:kmag] 2012-07-30 14:17:19 PDT
Everyone using the add-on is not very many people, and the developers are working on fixing it, so it's not currently a candidate for blocklisting.

They released an updated version a few days ago (which just hit AMO today) that seems to ameliorate the problem a bit, but it still uses mousemove and mouseleave event delegation, and I'm also seeing some extra CPU usage issues when the mouse is motionless, which I'm working on pinning down.
Comment 26 Alex Keybl [:akeybl] 2012-07-30 15:50:06 PDT
We're considering untracking this for release, based upon more user feedback now that 1.0.7 is released (since comments 24 and 25 are at odds).
Comment 27 Frank Freibuth 2012-07-30 16:03:55 PDT
I installed v1.0.7 in the current Aurora build. I didn't actually use the extension but used my Firefox normally.

Immediately after starting Firefox CPU-load is at ~8%, after about 2 hours one core is entirely maxed out again by firefox.exe

In short: While the devs claim to have addressed the issue in v1.0.7, I personally see absolutely no difference in behavior between v1.0.6 and v1.0.7 - the way the described bug manifests in both is absolutely identical from an end user's point of view.

v1.0.7 most certainly doesn't fix the problem for me, it changes nothing.
Comment 28 Jorge Villalobos [:jorgev] 2012-07-30 17:30:03 PDT
(In reply to Alex Keybl [:akeybl] from comment #26)
> We're considering untracking this for release, based upon more user feedback
> now that 1.0.7 is released (since comments 24 and 25 are at odds).

Judging by previous comments, this isn't really a Firefox regression. The add-on is certainly doing things wrong, and version 1.0.7 is a minor performance fix that doesn't address the root of the problem (mouse move listeners for the entire page). We'll continue working with the developers to get the problem resolved, but I agree that this isn't worth tracking for release.
Comment 29 Kris Maglione [:kmag] 2012-07-30 18:09:41 PDT
I agree that this is a problem with the add-on. It's interesting, though, that people are reporting it's considerably worse on 14 than on 13, which seems to suggest that something changed in Firefox to make things worse.
Comment 30 Alex 2012-07-31 09:01:33 PDT
In 1.0.7, the "hovering" option cannot be disabled.
Comment 31 Alex Keybl [:akeybl] 2012-07-31 11:25:38 PDT
(In reply to Jorge Villalobos [:jorgev] from comment #28)
> Judging by previous comments, this isn't really a Firefox regression. The
> add-on is certainly doing things wrong, and version 1.0.7 is a minor
> performance fix that doesn't address the root of the problem (mouse move
> listeners for the entire page). We'll continue working with the developers
> to get the problem resolved, but I agree that this isn't worth tracking for
> release.

Thanks Jorge - glad to hear you guys will remain engaged with Google. No need to track for release, in that case.
Comment 32 bethtukr 2012-07-31 23:01:18 PDT
Looking at the source it binds mousemove and removes then reinjects the the icon every time the mouse moves a pixel. Over and over and over. 

I assume that mouseenter doesn't work for some reason or that would have been used instead. At the very least add a check to the top of the handler after setting currentImage so it doesn't do any more work than necessary.
Not 100% but I think this would work in jquery:
if ( currentImage.is(imagesearch.sbi.active_image) ) return;
Comment 33 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-08-01 03:18:47 PDT
(mouseenter/mouseleave are always a bit slower than other mouse events, and shouldn't be
used unless really needed.  https://developer.mozilla.org/en/DOM/DOM_event_reference/mouseenter
mouseover and mouseout works usually faster.)
Comment 34 Jorge Villalobos [:jorgev] 2012-08-03 09:15:56 PDT
Quick update: so far the developers have submitted 2 updates, both of which have made minor performance improvements but haven't fixed the root of the problem.  Kris reviewed the latest version and gave them specific instructions on what needs to be corrected. We expect a new update early next week.
Comment 35 Alex 2012-08-03 10:06:23 PDT
1.08 is out
Comment 36 Jorge Villalobos [:jorgev] 2012-08-10 08:54:57 PDT
The add-on has been blocklisted and we won't accept any versions until the performance issues have been resolved. There's nothing more to do here.

Note You need to log in before you can comment on or make changes to this bug.