Bug 3169 - threaded message list performance issue
Summary: threaded message list performance issue
Status: RESOLVED FIXED
Alias: None
Product: Claws Mail (GTK 2)
Classification: Unclassified
Component: UI/Message List (show other bugs)
Version: 3.9.3
Hardware: PC Linux
: P3 normal
Assignee: users
URL:
Depends on:
Blocks:
 
Reported: 2014-05-15 11:20 UTC by Igor Mammedov
Modified: 2014-05-29 14:31 UTC (History)
0 users

See Also:


Attachments
callgrind sample file (355.01 KB, application/octet-stream)
2014-05-20 09:49 UTC, Igor Mammedov
no flags Details

Description Igor Mammedov 2014-05-15 11:20:13 UTC
When using "Next unread message" in "Thread view" mode it sometimes takes
10-15sec till next unread message is displayed.

Running claws-mail under valgrind shows that problem is in function tree_draw_node(), which is called 500K times in my case on one "Next unread message" execution.
It has loop:

       while (work && work != node)
        {
          work = GTK_CMCTREE_NODE_NEXT (work);
          num++;
        }

which ultimately leads to O(n^2) complexity.

I don't know claws-mail code enough to fix it but question is: could 'num' variable be cached in GtkCMCTreeNode when tree is constructed
to avoid above loop?


Conditions to reproduce (which sometimes trigger issue):
 Folder with a big amount of emails (~100K) which are heavily threaded in "Thread View" mode. All email mostly read and some new mail arrived to some old threads.
Comment 1 Colin Leroy 2014-05-15 21:02:28 UTC
Hi,

Can you provide a full backtrace of this ? This is abnormal.
Comment 2 Igor Mammedov 2014-05-20 09:49:31 UTC
Created attachment 1371 [details]
callgrind sample file

Attached a sample taken with callgrind tool at the moment of issue.
I've used kcachegrind GUI tool to explore it.
Let me know if file works for you and you can use it.

There is no nice stack trace. it's called from gdk_event_dispatch()
here is top 2 callers of tree_draw_node():
 73% 356000times gtk_cmtree_node_set_text()
 18%  66000times gtk_cmtree_node_set_foreground()

the most expensive LOC in tree_draw_node():
 90%inclusive  "while (work && work != node)"

which calculates num index inside tree iterating over tree
which leads to computational cost explosion.
Comment 3 Colin Leroy 2014-05-20 10:40:33 UTC
Thanks! Can you also include a --debug log ?

Thanks in advance.
Comment 4 users 2014-05-20 10:59:03 UTC
Changes related to this bug have been committed.
Please check latest Git and update the bug accordingly.
You can also get the patch from:
http://git.claws-mail.org/

++ ChangeLog	2014-05-20 10:59:03.507381768 +0200
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=518c6d5595a927646226ecad7332f2610cf03008
Merge: 746c57f aadd04d
Author: Colin Leroy <colin@colino.net>
Date:   Tue May 20 10:59:02 2014 +0200

    Merge branch 'master' of file:///home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=aadd04da6020494c61862cb5154c99fd4bca798f
Author: Colin Leroy <colin@colino.net>
Date:   Tue May 20 10:58:34 2014 +0200

    Possibly fix bug #3169, threaded message list performance issue
Comment 5 Colin Leroy 2014-05-20 11:03:11 UTC
Hi,

The patch I just commited should help. Caching num in the GtkCMCTreeNode struct would help in this case, but has the two drawbacks of doubling the needed memory, and needing re-calculation of every node after a newly inserted one when they're not inserted at the end (and they're not :-))
Comment 6 Igor Mammedov 2014-05-20 11:57:05 UTC
(In reply to comment #3)
> Thanks! Can you also include a --debug log ?
> 
> Thanks in advance.

I'm sorry, but no log available and problematic mailbox that causes issue is read now.
Comment 7 Igor Mammedov 2014-05-20 11:57:59 UTC
(In reply to comment #3)
> Thanks! Can you also include a --debug log ?
> 
> Thanks in advance.

I'm sorry, but no log available and problematic mailbox that causes issue is read now.
(In reply to comment #4)
> Changes related to this bug have been committed.
> Please check latest Git and update the bug accordingly.
> You can also get the patch from:
> http://git.claws-mail.org/
> 
> ++ ChangeLog	2014-05-20 10:59:03.507381768 +0200
> http://git.claws-mail.org/?p=claws.git;a=commitdiff;
> h=518c6d5595a927646226ecad7332f2610cf03008
> Merge: 746c57f aadd04d
> Author: Colin Leroy <colin@colino.net>
> Date:   Tue May 20 10:59:02 2014 +0200
> 
>     Merge branch 'master' of file:///home/git/claws
> 
> http://git.claws-mail.org/?p=claws.git;a=commitdiff;
> h=aadd04da6020494c61862cb5154c99fd4bca798f
> Author: Colin Leroy <colin@colino.net>
> Date:   Tue May 20 10:58:34 2014 +0200
> 
>     Possibly fix bug #3169, threaded message list performance issue

I'll give it a try.
Comment 8 Colin Leroy 2014-05-29 13:58:33 UTC
Hi,

Can you confirm the fix?

Thanks!
Comment 9 Igor Mammedov 2014-05-29 14:31:05 UTC
(In reply to comment #8)
> Hi,
> 
> Can you confirm the fix?
> 
> Thanks!

Patch to me looks more like work-around, but it fixes issue for me.
Thanks for it.

---
PS:
I have not right to complain about current impl.,
since I'm not providing path and have no time for fixing
it so that there weren't O(n^2) complexity there.

Note You need to log in before you can comment on or make changes to this bug.