Closed
Bug 955675
Opened 10 years ago
Closed 10 years ago
Don't keep recalculating possibleChat ids
Categories
(Instantbird Graveyard :: Conversation, defect)
Instantbird Graveyard
Conversation
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: aleth, Assigned: aleth)
References
Details
Attachments
(1 file, 2 obsolete files)
3.66 KB,
patch
|
nhnt11
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2227 at 2013-10-18 15:08:00 UTC *** Is there a reason the id on possibleChats is not a lazy getter? http://lxr.instantbird.org/instantbird/source/instantbird/components/ibConvStatsService.js#669 The profiler seems to suggest we spend quite some time there (in particular due to the this.account call).
Comment 1•10 years ago
|
||
*** Original post on bio 2227 at 2013-10-18 16:02:42 UTC *** Couldn't we instead avoid calling this.id in: 736 get computedScore() { 737 let stats = gStatsByConvId[this.id]; Maybe it would be less expensive at the end of LIST results for an account to go through the list of convs of the account that have stats (I expect that to be a small number), and link these to the LIST results?
Assignee | ||
Comment 2•10 years ago
|
||
*** Original post on bio 2227 as attmnt 2996 at 2013-10-30 10:44:00 UTC *** I'll leave the suggestion in comment 1 for nhnt11 (I think it's worth considering, but it would be a separate bug).
Attachment #8354777 -
Flags: review?(nhnt11)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Updated•10 years ago
|
Summary: Make possibleChat id a lazy getter → Don't keep recalculating possibleChat ids
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 2227 as attmnt 2997 at 2013-10-30 11:57:00 UTC *** Replace id getter with property.
Attachment #8354778 -
Flags: review?(nhnt11)
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8354777 [details] [diff] [review] Patch *** Original change on bio 2227 attmnt 2996 at 2013-10-30 11:57:52 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354777 -
Attachment is obsolete: true
Attachment #8354777 -
Flags: review?(nhnt11)
Assignee | ||
Comment 5•10 years ago
|
||
*** Original post on bio 2227 at 2013-10-30 12:43:01 UTC *** (In reply to comment #1) > Maybe it would be less expensive at the end of LIST results for an account to > go through the list of convs of the account that have stats (I expect that to > be a small number), and link these to the LIST results? Or "what if the possibleConvs didn't pull from the stats, but the stats pushed to the possibleConvs". Cf Bug 955630 (bio 2185) for another instance where the stats will have to push. This should be a separate bug if it is followed up.
Comment 6•10 years ago
|
||
Comment on attachment 8354778 [details] [diff] [review] Patch *** Original change on bio 2227 attmnt 2997 at 2013-10-30 22:42:33 UTC *** > function ExistingConversation(aUIConv) { > this._convId = aUIConv.target.id; > let account = aUIConv.account; >- this._id = getConversationId(account.protocol.normalizedName, >+ this.id = getConversationId(account.protocol.normalizedName, > account.name, aUIConv.normalizedName, > aUIConv.isChat); Looks like you wanted to re-indent these 2 lines. r-, but mostly because I want to check that bugzilla still sends emails to instantbot ;).
Attachment #8354778 -
Flags: review-
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 2227 as attmnt 2998 at 2013-10-31 10:17:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354779 -
Flags: review?(nhnt11)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8354778 [details] [diff] [review] Patch *** Original change on bio 2227 attmnt 2997 at 2013-10-31 10:17:35 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354778 -
Attachment is obsolete: true
Attachment #8354778 -
Flags: review?(nhnt11)
Comment 9•10 years ago
|
||
Comment on attachment 8354779 [details] [diff] [review] Patch *** Original change on bio 2227 attmnt 2998 at 2013-11-06 14:00:17 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354779 -
Flags: review?(nhnt11) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 10•10 years ago
|
||
*** Original post on bio 2227 at 2013-11-06 16:50:40 UTC *** http://hg.instantbird.org/instantbird/rev/70049aae9f19
Status: ASSIGNED → 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
•