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.
What do you call slow? Can you define this a bit more? The strike through definitely does not reduce performance.
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.
When deleting an IMAP message (which may take some seconds) the strike-thru is a nice visual indicator to have IMHO.
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).
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.
Can someone review the patch, please?
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:)
*** Bug 109178 has been marked as a duplicate of this bug. ***
*** Bug 123804 has been marked as a duplicate of this bug. ***
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.
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 ?
Can confirm with current 3.5.5 head
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() );