Closed Bug 462077 Opened 11 years ago Closed 11 years ago

Lazily create Extensions object

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 3.1b2

People

(Reporter: sdwilsh, Assigned: sdwilsh)

Details

Attachments

(1 file, 2 obsolete files)

Right now, we always create the Extensions() object on extApplication when we receive the app-startup notification.  However, in a dtrace profile this takes 17 ms of time to do, and nothing in browser actually uses it.  We should lazily create this object with a smart getter so we do not have to take this hit on every startup.
Attached patch v1.0 (obsolete) — Splinter Review
Attachment #345202 - Flags: review?(gavin.sharp)
Comment on attachment 345202 [details] [diff] [review]
v1.0

<gavin|> sdwilsh: just don't initialize _extensions, and make the existing "extensions" getter smart
Attachment #345202 - Flags: review?(gavin.sharp) → review-
Attached patch v1.1 (obsolete) — Splinter Review
That last one was stupid.  This makes more sense.
Attachment #345202 - Attachment is obsolete: true
Attachment #345206 - Flags: review?(gavin.sharp)
Attached patch v1.2Splinter Review
how gavin really wants it
Attachment #345206 - Attachment is obsolete: true
Attachment #345212 - Flags: review?(gavin.sharp)
Attachment #345206 - Flags: review?(gavin.sharp)
Attachment #345212 - Flags: review?(gavin.sharp) → review+
Whiteboard: [has patch][has review][can land]
http://hg.mozilla.org/mozilla-central/rev/1e6548dbdcbc
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has review][can land]
Did you look at all into why creating it was so slow? It seems all it does is get a reference to the extension manager. If it is that that is taking time then that's happened is you've moved the EM init to slightly later in the startup process (literally moments later, it gets created during app-startup too)
No - now it shouldn't ever get created unless someone uses it (we don't).
Dave's point is that the creation of the Extensions object itself is trivial. The only work it does is instantiate the EM, which gets instantiated soon after even with this patch. It doesn't hurt to remove (or delay) getService() calls, of course, but there isn't really any evidence to believe that this will have had any impact on startup.
It had a measurable impact.  See bug 459197 comment 26.
I find it pretty hard to see a significant drop there; looks to me like it's within the noise (especially when you look at more than 2 or 3 runs).
Well that somewhat depends on how you interpret the graphs. To me it appears that Vista and XP all showed a short period of Ts improvement, starting shortly before this patch landed (about 10am) and ending shortly after (about 10pm). Linux doesn't really seem to show any Ts improvement till around 10am the next day.

The point is if there was a real impact to this then it seems worth trying to understand why. Maybe the locks surrounding getService calls really are more expensive than we'd like? I wonder what the speed of creating the Extensions object post startup is.
You need to log in before you can comment on or make changes to this bug.