Ticket #9703 (closed patch: fixed)

Opened 8 months ago

Last modified 8 months ago

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

conv-cache.patch (5.2 kB) - added by tmm1 8 months ago.
conv-cache2.patch (5.3 kB) - added by tmm1 8 months ago.

Change History

Changed 8 months ago by tmm1

Changed 8 months ago by darkrain42

  • owner set to darkrain42
  • milestone set to Patches Needing Review

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

Changed 8 months 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.

Changed 8 months 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 8 months ago by tmm1

Changed 8 months ago by aman@…

  • status changed from new to closed
  • resolution set to fixed
  • milestone changed from Patches Needing Improvement to 2.6.0

(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.