Closed Bug 955675 Opened 10 years ago Closed 10 years ago

Don't keep recalculating possibleChat ids

Categories

(Instantbird Graveyard :: Conversation, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

References

Details

Attachments

(1 file, 2 obsolete files)

*** 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).
Blocks: 955013
*** 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?
Attached patch Patch (obsolete) — Splinter Review
*** 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: nobody → aleth
Status: NEW → ASSIGNED
Summary: Make possibleChat id a lazy getter → Don't keep recalculating possibleChat ids
Attached patch Patch (obsolete) — Splinter Review
*** 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)
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)
*** 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 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-
Attached patch PatchSplinter Review
*** 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)
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 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+
Whiteboard: [checkin-needed]
*** 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.