Opened 10 years ago

Closed 10 years ago

#9703 closed patch (fixed)

add a GHashTable cache for conversations to speed up find_conversation_with_account

Reported by: tmm1 Owned by: darkrain42
Milestone: 2.6.0 Component: libpurple
Version: 2.5.8 Keywords:
Cc:

Description

conversations are stored as a linked list, which makes purple_find_conversation_with_account very expensive when you have a lot of conversations

Attachments (2)

conv-cache.patch (5.2 KB) - added by tmm1 10 years ago.
conv-cache2.patch (5.3 KB) - added by tmm1 10 years ago.

Download all attachments as: .zip

Change History (6)

Changed 10 years ago by tmm1

comment:1 Changed 10 years ago by darkrain42

  • Milestone set to Patches Needing Review
  • Owner set to darkrain42

A weird quirk exists in purple_conversation_new where it will create a new chat for the same (account, name) pair if the user hasn't left that room already. That needs to be resolved in order to use this hash table.

From #pidgin, nosnilmot contributed:

10:51:00 < nosnilmot> darkrain42: (replying to some comment you made yesterday 
                      about purple_conversation_new) a0cc5861 included cleanup 
                      to purple_conversation_new, which was reverted in 
                      65f3bcd3, there's certainly a reason for the odd behavior 
                      but I can't remember exactly what it is - possibly 
                      related to MSN's unnamed chats
10:52:54 < deryni> nosnilmot: Yeah, I saw the back and forth commits for it but 
                   they didn't provide enough context by themselves for me to 
                   figure it out.
10:53:34 < nosnilmot> deryni: I vaguely remember some discussion about it at 
                      the time, but can't remember where the discussion took 
                      place :(
10:55:45 < deryni> I actually wondered whether that odd behaviour was the cause 
                   of some of the yahoo and MSN multiple chat window problems 
                   we've had reported.
10:56:27 < deryni> But yeah, it would make sense for that to have been a 
                   workaround for MSN unnamed chats if we always use the same 
                   name for them.
10:56:44 < deryni> In which case I'd argue we should make the prpl stop doing 
                   that and remove the workaround.
11:04:04 < nosnilmot> deryni / darkrain42: 
                      http://pidgin.im/pipermail/devel/2008-January/004653.html
11:05:48 < deryni> nosnilmot: That looks like if we made MSN give chats unique 
                   names we'd be ok without the odd code, right?
11:06:21 < nosnilmot> deryni: probably, but it would look uglier
11:06:29 < nosnilmot> not the code, the UI. code would be cleaner :)
11:06:49 < deryni> Could we use conversation titles to avoid that?
11:10:17 < nosnilmot> conversation titles? aren't they the same as the chat 
                      name? I have been under a rock for too long
11:10:36 < deryni> I was sort of hoping you'd know what they were, since I 
                   don't offhand. =)

The revisions referenced are a0cc58611eba803e55891cb8cfefa80d29a8413b and 65f3bcd3e7dfafc090e0cdfa3e7fff40d82e3c9c and link is http://pidgin.im/pipermail/devel/2008-January/004653.html

comment:2 Changed 10 years ago by darkrain42

  • Milestone changed from Patches Needing Review to Patches Needing Improvement

tmm1, on/around line 751 (in purple_conversation_set_name), hc->name needs to be updated before inserting the updated PurpleConversation? into the blist.

comment:3 Changed 10 years ago by tmm1

I made the following change to the patch:

--- a/libpurple/conversation.c
+++ b/libpurple/conversation.c
@@ -748,7 +748,8 @@ purple_conversation_set_name(PurpleConversation *conv, const char *name)
 	g_hash_table_remove(conversation_cache, hc);
 	g_free(conv->name);
 
-	conv->name = g_strdup(purple_normalize(conv->account, name));
+	conv->name = g_strdup(name);
+	hc->name = g_strdup(purple_normalize(conv->account, conv->name));
 	g_hash_table_insert(conversation_cache, hc, conv);
 
 	purple_conversation_autoset_title(conv);

Changed 10 years ago by tmm1

comment:4 Changed 10 years ago by aman@…

  • Milestone changed from Patches Needing Improvement to 2.6.0
  • Resolution set to fixed
  • Status changed from new to closed

(In 7624afb13d9554cac3283113c46ff21a2438f2ae):
More efficient purple_find_conversation_with_account. Closes #9703.

Patch mostly from Aman "tmm1" Gupta; I added the g_list_prepend calls. The functionality is going to get whacky when dealing with chats that share the same name ("MSN Chat"), but purple_find_conversation_with_account doesn't work for those in any sense of the word 'work', so...

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!