Open Bug 68074 Opened 19 years ago Updated 6 months ago
Lots of Create
Instances of ns Event Listener Managers at startup
Some analaysis of an XPCOM log for startup and display of a simple HTML page in a Mozilla browser window shows that we create 1054 instances of nsEventListenerManagers. This seems a lot, and perhaps could be optimized.
Most of these nsEventListenerManagers seem to be created by nsXULElement's call to element->AddScriptEventListener(). At 80 bytes a pop, this is quite a space hog too.
Is our XUL using a bunch of onclick attributes, or some such? What's further up the stack in these calls to AddScriptEventListener? /be
Or worse, a XUL template with event handlers inside the <template>? alecf/ben/mcafee, you should do a pass through navigator.xul and its overlays. (sspitzer, same for mail.) Search for the `datasources' attribute; make sure that there are no event handlers contained inside a <template>.
found a bunch of badness in the mailnews xul. mailWindowOverlay.xul oncommand="MsgMoveMessage(event.target)" oncommand="MsgCopyMessage(event.target)" oncommand="MsgGetMessagesForAccount(event)" messengercompose.xul oncommand="MessageFcc(event.target)" msgFolderPickerOverlay.xul oncommand="PickedMsgFolder(event.target,'msgNewFolderPicker')" etc. (basically, every folder picker that I wrote is evil) FilterListDialog.xul onclick="onToggleEnabled(event);" pref-composing_messages.xul oncreate="dump('CREATE MENU NOW\n');" I'll go start a new bug to track the clean up of the mailnews xul. thanks for the heads up waterson / sfraser.
the mail xul is covered by #68174
Depends on: 68174
Here's my list: navigator.xul oncommand="OpenBookmarkURL(event.target,document.getElementById('BookmarksMenu').database)" /> oncommand="OpenBookmarkURL(event.target, document.getElementById('innermostBox').database);"/> oncommand="OpenBookmarkURL(event.target, document.getElementById('innermostBox').database)"/> navigatorOverlay.xul oncommand="selectLocale(event)" oncommand="applyTheme(event.target)" oncommand="OpenBookmarkURL(event.target, document.getElementById('BookmarksMenu').database)" internetresults.xul oncommand="doEngineClick(event, this);" search-editor.xul oncommand="return chooseCategory(this);" oncommand="chooseCategory(this);" oncommand="saveEngines();" sidebarOverlays.xul oncommand="SidebarTogglePanel(event.target);" charsetOverlay.xul oncommand="SelectDetectors( event )" taskbarOverlay.xul oncommand="rdf:http://home.netscape.com/NC-rdf#content" tasksOverlay.xul oncommand="ShowWindowFromResource(event.target)"
ok here's what needs to be done: 1) you need to find the most logical container for the template - i.e. the <tree> or <menulist> - and move the onclick or oncommand handler there 2) look at the event handler code, and figure out if it was dependent on the command handler being in the child node.. look for references to event.target, and more importantly, test it out. 9 times out of 10, it's not dependent and you can just move the event handler.
mcafee, I'm giving this bug to you since you've been working on this
Assignee: saari → mcafee
slee: what I mumbled in the meeting was to replace the CreateInstance of the nsEventListenerManager with a call to NS_NewEventListenerManager if you can (i.e. if the things are being created in the content DLL. This avoids a trip through XPCOM.
I went through the browser files and found some cases where we could move oncommand stuff outside of templates. I tested each one of these by first breaking it (removing the command handler), then adding it back into the new place to restore the functionality. Some of the JS functions were used in other JS, and sometimes it looked like the JS needed the handler in the template code. I managed to get changes for navigator.xul, navigatorOverlay.xul, and sidebarOverlays.xul. Files I had trouble with: internetresults.xul : ? I couldn't understand how to test this search-editor.xul: Other JS usages of chooseCategory(), more changes needed. taskbarOverlay.xul: Didn't work tasksOverlay.xul: ShowWindowFromResource() not working on linux, couldn't test.
these changes look great - you don't need to add the comment about the handlers moving to the template - they shouldn't have been there in the first place, and everywhere else they're outside the template already sr=alecf on the latest patch if you remove those comments
ok thanks alec
2nd patch checked in.
nav triage team: Chris, are there still any remaining issues? Why is this bug still open?
This is open per smfr's comment about avoiding xpcom/dll trip
nav triage team: Marking nsbeta1+, mozilla0.9.2, reassigning to pchen
Assignee: mcafee → pchen
Target Milestone: --- → mozilla0.9.2
nav triage team: Marking p3 and mozilla0.9.3
Priority: -- → P3
Target Milestone: mozilla0.9.2 → mozilla0.9.3
QA contact updated
QA Contact: gerardok → madhur
nav triage team: Avoiding trip through xpcom not that critical. Marking nsbeta1- and future
Assignee: pchen → joki
Assignee: joki → saari
QA Contact: trix → ian
Attachment #27366 - Attachment is obsolete: true
Comment on attachment 27430 [details] [diff] [review] 2nd patch, resolves cvs conflicts [Checkin: Comment 25] (In reply to comment #16) > 2nd patch checked in. A (code and comment) modified version was checked in: [ 2006-09-13 22:46 mcafee%netscape.com mozilla/suite/browser/navigator.xul 1.278 2006-09-13 22:46 mcafee%netscape.com mozilla/suite/browser/navigatorOverlay.xul 1.179 2006-07-27 07:47 mcafee%netscape.com mozilla/suite/common/sidebar/sidebarOverlay.xul 1.56 ] NB: The (2006) timestamps are unexpected ... There seems to be something wrong with Bonsaï (for this, and other, checkins) :-<
Attachment #27430 - Attachment description: 2nd patch, resolves cvs conflicts → 2nd patch, resolves cvs conflicts [Checkin: Comment 25]
Assignee: saari → nobody
Priority: P3 → --
QA Contact: ian → events
Target Milestone: Future → ---
blocking1.9=?: not mandatory, but as it is marked as 'perf' and seems easy enough to do...
(In reply to comment #26) > blocking1.9=?: > not mandatory, but as it is marked as 'perf' and seems easy enough to do... > What is left to do here? Likely not going to block but would *happily* take a patch.
(In reply to comment #27) > What is left to do here? I don't know precisely yet: see comment 12 and bug 68174 comment 19...
When we know what we want you can re-nom but not clear to me we should block on this
Flags: blocking1.9? → blocking1.9-
Currently when starting FF, ~320 ELMs are created. This seems to be in wrong component. Firefox-generic might be a better one, if the count of event listener attributes in UI should be reduced?
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.