Last Comment Bug 750583 - FUEL gShutdown array uses an n^2 algorithm to empty the array, causing huge shutdown times
: FUEL gShutdown array uses an n^2 algorithm to empty the array, causing huge s...
Status: RESOLVED FIXED
[snappy][qa+]
: perf
Product: Core
Classification: Components
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Patrick Walton (:pcwalton)
:
Mentors:
: 750469 (view as bug list)
Depends on:
Blocks: 750454 750953
  Show dependency treegraph
 
Reported: 2012-04-30 18:51 PDT by Patrick Walton (:pcwalton)
Modified: 2012-06-02 06:08 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
verified
fixed
fixed
wontfix


Attachments
Proposed patch. (1.10 KB, patch)
2012-04-30 18:51 PDT, Patrick Walton (:pcwalton)
justin.lebar+bug: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑esr10-
Details | Diff | Review

Description Patrick Walton (:pcwalton) 2012-04-30 18:51:06 PDT
Created attachment 619808 [details] [diff] [review]
Proposed patch.

For months I've been getting minute-long shutdown times after using the browser for a long time. I managed to turn up the following culprit in FUEL:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/exthelper/extApplication.js#594

Which is an n^2 loop for clearing out an array. Indeed most of the cost is in memmove.

The question is why this array is getting so big. I suspect it's an addon using FUEL inappropriately. In the meantime, at least we can speed up the emptying of the array.

Apologies if this is misfiled, as I have no idea who maintains FUEL...
Comment 1 Boris Zbarsky [:bz] 2012-04-30 19:05:07 PDT
> The question is why this array is getting so big.

Because FUEL's management of this array is totally broken.  See bug 750454 which bent filed today.  The patch here helps the memmove issue, but not the memory leaks FUEL is causing here.
Comment 2 Patrick Walton (:pcwalton) 2012-04-30 19:07:48 PDT
Duping over to bug 750454.

*** This bug has been marked as a duplicate of bug 750454 ***
Comment 3 Patrick Walton (:pcwalton) 2012-04-30 19:14:55 PDT
Unduping per #content.
Comment 4 Justin Lebar (not reading bugmail) 2012-05-01 07:57:30 PDT
*** Bug 750469 has been marked as a duplicate of this bug. ***
Comment 5 Patrick Walton (:pcwalton) 2012-05-04 12:12:12 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/139e8e845aa5
Comment 6 Ed Morley [:emorley] 2012-05-05 03:41:05 PDT
https://hg.mozilla.org/mozilla-central/rev/139e8e845aa5
Comment 7 Justin Lebar (not reading bugmail) 2012-05-08 16:31:40 PDT
Comment on attachment 619808 [details] [diff] [review]
Proposed patch.

[Approval Request Comment]
Case for branch / ESR consideration: Bad bug (causes very long shutdown times), trivial fix.
Fix Landed on Version: FF15
Risk to taking this patch (and alternatives if risky): Trivial change.
String changes made by this patch: None.
Comment 8 Lukas Blakk [:lsblakk] use ?needinfo 2012-05-09 15:44:54 PDT
Comment on attachment 619808 [details] [diff] [review]
Proposed patch.

happy to get these perf wins on beta/aurora, but denying for ESR since there's no current requests for this with the enterprise users.
Comment 9 Justin Lebar (not reading bugmail) 2012-05-09 15:55:30 PDT
(In reply to Lukas Blakk [:lsblakk] from comment #8)
> happy to get these perf wins on beta/aurora, but denying for ESR since
> there's no current requests for this with the enterprise users.

That's OK, I don't care about the enterprise.  :-P

https://hg.mozilla.org/releases/mozilla-aurora/rev/fb8127fcaac4
https://hg.mozilla.org/releases/mozilla-beta/rev/303d987789c5

QA: If you'd like to verify this fix, you're probably best off installing an old version of the Readibility add-on (bug 730546) and trying to coax it into creating a lot of objects.  Things like switching tabs create new objects with the old Readability.  I don't have a solid test-case; you will have to experiment, unless pcwalton has better STR.
Comment 10 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-29 11:24:20 PDT
We'll try to verify this using the suggestions in comment 9.
Comment 11 Mihaela Velimiroviciu (:mihaelav) 2012-05-30 08:03:01 PDT
Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20100101 Firefox/13.0

Used the steps from comment #9, but could not reproduce the issue: Firefox closed normally.

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