Closed
Bug 107481
Opened 23 years ago
Closed 15 years ago
Opening folder: Doing UpdateCommands("mail-toolbar") several times
Categories
(SeaMonkey :: MailNews: Message Display, defect)
SeaMonkey
MailNews: Message Display
Tracking
(Not tracked)
RESOLVED
EXPIRED
People
(Reporter: cavin, Unassigned)
References
Details
(Keywords: perf)
Attachments
(3 obsolete files)
This is filed against folder loading performance. Scenario: Start up mailnews program and then open( click on) a local folder with 1000 msgs. I started out by adding some timing code to java function ChangeFolderByURI() in base\resource\contgent\commandglue.js to see where time was spent when opening a folder and here is what I got: ++++++++++++++++++++++++++++++++++++++++++++++++++++++ In ChangeFolderByURI uri = mailbox://nobody@Local%20Folders/Big1000Msgs sortType = 18 *** Performance check #2: time used = 10 *** Performance check #3: time used = 10 *** Performance check #4: time used = 10 *** Performance check #5: time used = 20 *** Performance check #6: time used = 20 It takes 237 milliseconds for nsMsgLocalMailFolder::UpdateFolder() to finish *** Performance check #7: time used = 260 WEBSHELL- = 4 (150 milliseconds is used here) *** Performance check #11: time used = 410 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ It takes 150 milliseconds between 'Performance check #7' and 'Performance check #11' and the time was spent on the following statement: document.commandDispatcher.updateCommands('mail-toolbar'); where 'updateCommands' is defined in content/xul/document/src/nsXULCommandDispatcher.cpp (as nsXULCommandDispatcher::UpdateCommands()). Note that 'Performance check #11' gets printed out before UpdateCommands() returns (ie, the last statement of the function). Then I added some more code to nsXULCommandDispatcher::UpdateCommands() to figure out what it's doing and here is what I got: ++++++++++++++++++++++++++++++++++++++++++++++++++++++ In ChangeFolderByURI uri = mailbox://nobody@Local%20Folders/Big1000Msgs sortType = 18 *** Performance check #2: time used = 10 *** Performance check #3: time used = 10 *** Performance check #4: time used = 20 *** Performance check #5: time used = 30 *** Performance check #6: time used = 30 It takes 260 milliseconds for nsMsgLocalMailFolder::UpdateFolder() to finish *** Performance check #7: time used = 291 Doing UpdateCommands("mail-toolbar") UpdateCommands point #1: 4 WEBSHELL- = 4 UpdateCommands point #2: 152 UpdateCommands point #3: 153 *** Performance check #11: time used = 451 Doing UpdateCommands("focus") UpdateCommands point #1: 2 Doing UpdateCommands("mail-toolbar") UpdateCommands point #1: 1 UpdateCommands point #2: 91 UpdateCommands point #3: 92 UpdateCommands point #2: 206 UpdateCommands point #2: 241 UpdateCommands point #3: 242 Doing UpdateCommands("focus") UpdateCommands point #1: 1 Doing UpdateCommands("mail-toolbar") UpdateCommands point #1: 1 UpdateCommands point #2: 107 UpdateCommands point #3: 108 UpdateCommands point #2: 204 UpdateCommands point #2: 241 UpdateCommands point #3: 242 Doing UpdateCommands("focus") UpdateCommands point #1: 1 UpdateCommands point #2: 7 UpdateCommands point #2: 41 UpdateCommands point #3: 42 Doing UpdateCommands("focus") UpdateCommands point #1: 1 UpdateCommands point #2: 7 UpdateCommands point #2: 55 UpdateCommands point #3: 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ Here, we can tell that between 'Performance check #7' and 'Performance check #11' we issued 1 UpdateCommands("mail-toolbar") call and immediately after 'Performance check #11' another 2 UpdateCommands("mail-toolbar") calls were invoked along with 4 UpdateCommands("focus") calls. The first UpdateCommands("mail-toolbar") call is invoked by the java function ChangeFolderByURI() so this is expected. But the two extra UpdateCommands("mail-toolbar") calls which were triggered by the associated UpdateCommands("focus") calls was somehting I did not expect to see and they take up 242*2= 484 milliseconds! The question is do we really need all 3 UpdateCommands("mail-toolbar") calls? If we can just save one or two such calls here we'll be gaining a big percentage in folder opening time. One thing to note is that the execution time for UpdateCommands("mail-toolbar") calls seem to be very stable and is not affected by the size of the folder. Another side note here: UpdateCommands("focus") triggers UpdateCommands("mail-toolbar") call because in nsXULCommandDispatcher::UpdateCommands() it calls content->HandleDOMEvent() which eventually invokes UpdateCommands("mail-toolbar") (ie, recursive).
Updated•23 years ago
|
Comment 1•23 years ago
|
||
cavin's found good stuff. here's the worst case scenario: you've got a message selected in your imap inbox, focus in your thread pane, you click on your imap sent folder. here's what happens: 1) clicking on the folder pane does a focus switch, bluring focus from thread pane to folder pane. 2) during the blur, we set the focused element to null (update toolbars #1), then we set it to the folder pane (update toolbars #2) 3) the selection in the thread pane goes from 1 to 0, (update toolbars #3). nsMsgDBView.cpp, SelectionChanged() mCommandUpdater->UpdateCommandStatus(); 4) folder loads, (update toolbars #4)commandglue.js, ChangeFolderByURI(): document.commandDispatcher.updateCommands('mail-toolbar'); 5) folder is done loading, we switch focus to the thread pane automatically. that's another blur. (update toolbars #5, from focused element to null, and #6 when focused element goes to the thread pane) that's 6 times we update the toolbar on a single user action. cavin and I have a fix in hand for 2 of these, we've fixed the blur problem. we'll work on the others next. I think we can get down to 2. two focus switches: thread pane to folder pane, and then folder pane back to thread pane.
Updated•23 years ago
|
Summary: Opening folder: Doing UpdateCommands("mail-toolbar") 3 times? → Opening folder: Doing UpdateCommands("mail-toolbar") several times
Comment 2•23 years ago
|
||
Reporter | ||
Comment 3•23 years ago
|
||
Comment on attachment 55850 [details] [diff] [review] patch for the blur problem. r=cavin.
Attachment #55850 -
Flags: review+
Reporter | ||
Comment 4•23 years ago
|
||
With the patch, we save one updateCommands('mail-toolbar') call which is about 250 millisceconds for the scenario I've been testing against. A very nice gain here. Update on the scenario I've been using: Don't have any imap or pop account. The only thing the profile has is the local mail folders. So I normally start up the mailnews program and then click on the folder with 1000 (or 20000/4000) msgs.
Comment 5•23 years ago
|
||
Comment on attachment 55850 [details] [diff] [review] patch for the blur problem. sr=bienvenu
Attachment #55850 -
Flags: superreview+
Comment 6•23 years ago
|
||
ok, that patch is in. I'll continue working on this, to supress our updates of the toolbar even further.
Reporter | ||
Comment 7•23 years ago
|
||
With this 'blur problem' patch, we save 2 UpdateCommands("mail-toolbar") calls for the following (more normal) scenario (which is what seth described earlier as the worst case scenario): 1. Start program and log onto an imap acct so the focus is on INBOX thread pane. 2. Then click on a local folder. Without the patch we invoke 6 UpdateCommands("mail-toolbar") calls and only 4 with the patch.
Comment 8•23 years ago
|
||
For what it's worth, command updaters have a |targets| attribute that is specifically for this purpose -- designating for what elements you want to watch certain events (e.g. blur) for, so you don't have to do it yourself. The attribute takes a comma-delimited list of ids.
Comment 9•23 years ago
|
||
Comment 11•23 years ago
|
||
Updated•23 years ago
|
Attachment #55850 -
Attachment is obsolete: true
Updated•23 years ago
|
Attachment #56027 -
Attachment is obsolete: true
Comment 12•23 years ago
|
||
Comment on attachment 56029 [details] [diff] [review] new patch, update comments. patch also includes spelling fix (suppress, not supress) sr=bienvenu
Attachment #56029 -
Flags: superreview+
Comment 13•23 years ago
|
||
after that patch, here's what's left: task # of calls to updating toolbar startup 2 switching local folders, nothing selected 2 switching local folders, message selected 2 switching imap folders, nothing selected 2 switching imap folders, message selected 2 switching folders if switching folder to an account that ends up loading the start page 3 after this lands, I'll worry about reducing the updates even further. but before I start that, I'll go investigate what blake said, to simplfy the first patch.
Status: NEW → ASSIGNED
Reporter | ||
Comment 14•23 years ago
|
||
Comment on attachment 56029 [details] [diff] [review] new patch, update comments. patch also includes spelling fix (suppress, not supress) r=cavin.
Attachment #56029 -
Flags: review+
Comment 16•23 years ago
|
||
Comment on attachment 56029 [details] [diff] [review] new patch, update comments. patch also includes spelling fix (suppress, not supress) checked in, so marking obsolete.
Attachment #56029 -
Attachment is obsolete: true
Comment 17•23 years ago
|
||
moving to 0.9.7, not a blocker for 0.9.6
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Updated•23 years ago
|
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Comment 18•23 years ago
|
||
moving to future re: conversation I had earlier with Seth.
Target Milestone: mozilla0.9.8 → Future
Updated•23 years ago
|
Comment 19•22 years ago
|
||
Has this bug been fixed? I have a problem with Mozilla hanging when entering a folder and am looking for appropriate bug.
Updated•20 years ago
|
Product: Browser → Seamonkey
Updated•20 years ago
|
Assignee: sspitzer → mail
Status: ASSIGNED → NEW
Updated•16 years ago
|
Assignee: mail → nobody
Priority: P1 → --
QA Contact: stephend → message-display
Target Milestone: Future → ---
![]() |
||
Comment 20•15 years ago
|
||
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state. If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way. If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar). If no action happens within the next few months, we move this bug report to an EXPIRED state. Query tag for this change: mass-UNCONFIRM-20090614
Status: NEW → UNCONFIRMED
![]() |
||
Comment 21•15 years ago
|
||
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state. If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way. If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar). If no action happens within the next few months, we move this bug report to an EXPIRED state. Query tag for this change: mass-UNCONFIRM-20090614
Group: core-security
![]() |
||
Comment 22•15 years ago
|
||
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state. If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way. If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar). If no action happens within the next few months, we move this bug report to an EXPIRED state. Query tag for this change: mass-UNCONFIRM-20090614
![]() |
||
Updated•15 years ago
|
Group: core-security
![]() |
||
Comment 23•15 years ago
|
||
MASS-CHANGE: This bug report is registered in the SeaMonkey product, but still has no comment since the inception of the SeaMonkey project 5 years ago. Because of this, we're resolving the bug as EXPIRED. If you still can reproduce the bug on SeaMonkey 2 or otherwise think it's still valid, please REOPEN it and if it is a platform or toolkit issue, move it to the according component. Query tag for this change: EXPIRED-20100420
Status: UNCONFIRMED → RESOLVED
Closed: 15 years ago
Resolution: --- → EXPIRED
You need to log in
before you can comment on or make changes to this bug.
Description
•