Bug 87857 - Selecting an email and pressing delete is unnessarily slow
Summary: Selecting an email and pressing delete is unnessarily slow
Status: RESOLVED FIXED
Alias: None
Product: kmail
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: unspecified Linux
: NOR major with 1 vote (vote)
Target Milestone: ---
Assignee: kdepim bugs
URL:
Keywords:
: 109178 123804 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-08-23 17:53 UTC by Kevin DeKorte
Modified: 2007-09-14 12:17 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch fixing slow delete problem (1007 bytes, patch)
2006-03-30 22:20 UTC, Marek Janukowicz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin DeKorte 2004-08-23 17:53:44 UTC
Version:            (using KDE KDE 3.3.0)
Installed from:    Unlisted Binary Package
Compiler:           Package produced by kde-redhat
OS:                Linux

When using a local mail store and deleting a single email the delete process is slow. 

Steps:

1. select and email
2. press the delete key

Results

1. In message list the select message is drawn with a strike-thru text and then removed from the list. When deleting a single email the strike-thru step should be skipped to improve performance.
Comment 1 Carsten Burghardt 2004-08-23 20:52:46 UTC
What do you call slow? Can you define this a bit more?
The strike through definitely does not reduce performance.

Comment 2 Kevin DeKorte 2004-08-23 21:19:12 UTC
In the kmail in KDE 3.2 after pressing delete there was no delay. Now when I press delete the mail summary screen is redrawn twice, once to show the strike thru and once to remove the mail item from the list. Since I am just deleting one email it should just be removed from the list.  
Comment 3 Daniel Hahler 2006-01-05 19:02:24 UTC
When deleting an IMAP message (which may take some seconds) the strike-thru is a nice visual indicator to have IMHO.
Comment 4 Oded Arbel 2006-03-23 12:05:58 UTC
Indeed, but this also happens for locally stored email (under the "Local Folders" tree). Specifically, when deleting large amounts of email this is unbearably slow, and during the process the entire kmail window is unresponsive - a "wait" cursor is show and the screen does not repaint except for a flicker in the status panel.

I actually encountered this problem when I notices that my trash folder has over 15000 messages in it. When I tried to delete it kmail got stuck for more then 10 minutes until I killed it. I then proceeded to delete the mail manually from the maildir folder. During the time it was trying to delete, I straced and ltraced the process and it looked to me that it was taking its sweet time - something like 2 or 3 seconds for each message deleted. 

When trying to reproduce the problem I couldn't get kmail to be that slow, but in my tests kmail deletes only a few dozens messages a second (in a P3-1000). In that rate, a mail folder of several thousands messages (such as my trash folder about a few months without cleaning) will take over 10 minutes to clear, the whole time kmail is unresponsive (I usually take the time to make coffee ;-).

Here is what I did to reproduce this: 
- shutdown kmail
- make sure that at least 1 message is in the local trash folder.
- cd $HOME/.kde/share/apps/kmail/mail/trash/cur
- run this script to duplicate one of the message:
perl -le 'sub randChar { $out .= (a..z,A..Z)[int(rand()*52)] while length($out) < 5; $out;} for (1..10000) { print time().".".int(10000*rand()).".".randChar();}' | xargs -L1 ln -f <mail message to duplicate>
- start kmail, go to the local trash folder and instruct kmail to "empty trash".
- time it (a wall clock should be just fine - its very slow).
Comment 5 Marek Janukowicz 2006-03-30 22:20:45 UTC
Created attachment 15382 [details]
Patch fixing slow delete problem

I dug through the code and narrowed the problem to KMHeaders::prepareMove
method. When all messages from a folder were to be deleted, it scanned ALL
messages in the folder for EACH message deleted (O(n^2) complexity) just to set
the selection after delete job completed. 

I changed this to only scan next and previous message. In my opinion it should
be enough, as if next/previous message is selected too, it will also be deleted
(which in turn will fire prepareMove method again, selecting another
next/previous message).

I'm sure I didn't make myself clear enough, but if anyone acquainted with kmail
code reviews it, he should understand the idea.

It is my first patch against KDE (besides some work in KDevelop a long time
ago), so any comments are welcome.

The patch was made against /branches/KDE/3.5/kdepim from yesterday.
Comment 6 Marek Janukowicz 2006-04-06 15:25:45 UTC
Can someone review the patch, please?
Comment 7 Ismail Onur Filiz 2006-04-25 10:38:53 UTC
I had a look at your patch, but changing the function the way you do is a bit problematic, although I see where you are coming from. The msgRemoved slot calls prepareMove, which causes the O(n^2) problem you have indicated if all messages are selected (and thus prepareMove returns 0). Nevertheless, a few other functions also call this function, and the problem is they assume that prepareMove returns an unselected message if it returns anything non-zero. To obtain some visual artifacts with your patch for instance, say that there are 5 messages 1,2,3,4,5 in a folder. With ctrl, select 2,4,3 in that order, and move them to another folder with the right-click menu. The strike-throughs won't be properly drawn.

I'm thinking the following might be a better solution:
Index: kmheaders.cpp
===================================================================
--- kmheaders.cpp       (revision 532874)
+++ kmheaders.cpp       (working copy)
@@ -1457,8 +1457,9 @@ void KMHeaders::finalizeMove( HeaderItem
 {
   emit selected( 0 );

+  clearSelection();
+
   if ( item ) {
-    clearSelection();
     setCurrentItem( item );
     setSelected( item, true );
     setSelectionAnchor( currentItem() );

Since when all messages in a folder are selected, item will be zero, which in turn will clear the selection, which in turn should bypass the while loops in prepareMove in the next call. Nevertheless, I'm too sleepy right now to clearly think whether this is true, so I will leave some of the work to you Marek:)
Comment 8 Andreas Gungl 2006-11-30 09:40:59 UTC
*** Bug 109178 has been marked as a duplicate of this bug. ***
Comment 9 Andreas Gungl 2006-12-05 08:59:15 UTC
*** Bug 123804 has been marked as a duplicate of this bug. ***
Comment 10 konold 2006-12-11 09:34:06 UTC
On my 2.1 GHz, 1GB machine I cannot delete a 10000 messages spam folder. Therefore I increase the severity to major. IMHO a fix should go in the next release.
Comment 11 Oded Arbel 2006-12-11 09:46:02 UTC
I've noticed that this ticket is still unconfirmed (and now has severity major). If this is indeed a problem, can someone please mark this bug as confirmed ?
Comment 12 Martin Koller 2007-01-05 18:56:38 UTC
Can confirm with current 3.5.5 head
Comment 13 Martin Koller 2007-01-05 19:17:53 UTC
SVN commit 620346 by mkoller:

BUG: 87857

always clear selection in finalizeMove, especially important when there
will be no item for selection afterwards as this avoids O(n^2) in
prepareMove()
Patch from Ismail Onur Filiz


 M  +1 -1      kmheaders.cpp  


--- branches/KDE/3.5/kdepim/kmail/kmheaders.cpp #620345:620346
@@ -1476,9 +1476,9 @@
 void KMHeaders::finalizeMove( HeaderItem *item, int contentX, int contentY )
 {
   emit selected( 0 );
+  clearSelection();
 
   if ( item ) {
-    clearSelection();
     setCurrentItem( item );
     setSelected( item, true );
     setSelectionAnchor( currentItem() );