Closed Bug 777266 Opened 12 years ago Closed 12 years ago

Official "Search by Image (by Google)" v1.0.6 add-on causes high CPU utilization

Categories

(WebExtensions :: General, defect)

Product:
WebExtensions
Component:
General
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox15-, firefox16-)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox15 - ---
firefox16 - ---

People

(Reporter: freibooter, Unassigned)

References

Details

(Whiteboard: [Snappy][sps][3rd-party-bustage]) 

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.
Mozillazine forum discussion ==> http://forums.mozillazine.org/viewtopic.php?f=23&t=2506211
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Requesting tracking to get attention to this report.
tracking-firefox15: --- → ?
Component: Untriaged → Extension Compatibility
Keywords: regression
Whiteboard: [memshrink]
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
Whiteboard: [memshrink] → [Snappy]
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
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.
Component: Extension Compatibility → JavaScript Engine
Product: Firefox → Core
Summary: Official "Search by Image (by Google)" v1.0.6 add-on causes severe CPU leak. → Official "Search by Image (by Google)" v1.0.6 add-on causes high CPU utilization
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.
Component: JavaScript Engine → Add-ons
Product: Core → Tech Evangelism
Version: 15 Branch → unspecified
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?
(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?
(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.
That event listener is seriously expensive. I doubt very much that any changes Firefox-side are at fault: https://gist.github.com/3182602
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.
Yeah, the profile above indicated that the largest single chunk of time was being spent in Jquery.
Anyhow, regression range would be *very* useful.
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.
I had the same experience as Igor. Issue began with the upgrade to Firefox 14.
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)
(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.
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%.
Whiteboard: [Snappy] → [Snappy][sps]
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.
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?
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."
(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.
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.
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.
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).
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.
(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.
Keywords: regression, regressionwindow-wanted
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.
In 1.0.7, the "hovering" option cannot be disabled.
(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.
tracking-firefox15: + → -
tracking-firefox16: + → -
Whiteboard: [Snappy][sps] → [Snappy][sps][3rd-party-bustage]
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;
(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.)
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.
1.08 is out
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.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Component: Add-ons → General
Product: Tech Evangelism → WebExtensions
You need to log in before you can comment on or make changes to this bug.