Closed
Bug 955603
Opened 10 years ago
Closed 10 years ago
Displaying a large conversation log freezes the UI for a while before the first messages get displayed
Categories
(Instantbird Graveyard :: Conversation, defect)
Instantbird Graveyard
Conversation
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: florian, Assigned: florian)
Details
Attachments
(1 file, 1 obsolete file)
11.16 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2163 at 2013-09-07 22:46:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 2163 as attmnt 2850 at 2013-09-07 22:46:00 UTC *** With a large (1.3MB) IRC log, my debug build is frozen for 7s (my optimized build for 1s) between the click on the list item of the log, and the moment when the first message is displayed in the browser. This is due to us creating all the message objects at once in logger.js, and passing all of them through xpconnect at once as an array, and finally looping over all of them in viewlog.js to set nick colors. All of this can be done lazily. This is what the attached patch does.
Attachment #8354620 -
Flags: review?(aleth)
Comment 2•10 years ago
|
||
Comment on attachment 8354620 [details] [diff] [review] Patch *** Original change on bio 2163 attmnt 2850 at 2013-09-08 18:10:50 UTC *** This should indeed be a great improvement! The code itself looks r+, but there are now a lot of fields and methods, some internal and some not, with very similar names around message-adding and displaying, which is confusing at first glance. So I think we need some comments above the methods explaining when they are to be used/how they interrelate. E.g. for the getNextPendingMessage/getPendingMessageCount pair, mention that these should be overridden if messages are to be added via an enumerator rather than via appendMessage. > <field name="_displayPendingMessagesCalls">0</field> This field definition wants to stay above displayPendingMessages. >+ <field name="_pendingMessagesDisplayed">0</field> Needs a > <!-- These variables are reset in onStateChange. --> comment.
Attachment #8354620 -
Flags: review?(aleth) → review-
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 2163 as attmnt 2858 at 2013-09-09 22:30:00 UTC *** Interdiff: http://pastebin.instantbird.com/328228
Attachment #8354628 -
Flags: review?(aleth)
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8354620 [details] [diff] [review] Patch *** Original change on bio 2163 attmnt 2850 at 2013-09-09 22:30:15 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354620 -
Attachment is obsolete: true
Comment 5•10 years ago
|
||
Comment on attachment 8354628 [details] [diff] [review] Patch v2 *** Original change on bio 2163 attmnt 2858 at 2013-09-11 13:34:42 UTC *** Thanks! Before landing, can you move this >+ <!-- This variable is reset in onStateChange. --> >+ <field name="_nextPendingMessageIndex">0</field> directly above getNextPendingMessage as it is only used from the following two methods (to make it clear visually it belongs to those two overrideable methods).
Attachment #8354628 -
Flags: review?(aleth) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 6•10 years ago
|
||
*** Original post on bio 2163 at 2013-09-12 23:06:21 UTC *** http://hg.instantbird.org/instantbird/rev/3f23ff3747f8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
You need to log in
before you can comment on or make changes to this bug.
Description
•