Bug 151916

One way I can think to make it faster is this: Instead of storing mStatusChangedLocally for each folder (look in kmfoldercachedimap.cpp) as a boolean stating whether the folder has _ANY_ statuses changed, we could store an array of the uids that are changed so that when uploadFlags() is called we can only look at those particular uids. 

Created attachment 22009 [details]
Patch to only upload status of changed messages

I briefly tested this and it appears to work as expected. The debugging
messages confirm that only the status of the changed messages is uploaded.

It should probably be changed to store the UID rather than just the index in
the message folder because that will be the same after new messages arrive.

I tested the patch from Comment #2 on my system and it appears to work. Thanks Bonne!

*** This bug has been confirmed by popular vote. ***

I would like to use the patch in comment #2. But I don't know how to install it. It seems to be be portage specific (see the first two lines). Could someone write a short note how to use the patch with other distributions? I am a user of openSUSE and Kubuntu Feisty.

Thanks a lot,
niwi

I have written earlier today about my problems to install the patch in comment#2. I think I finally succeeded in installing the patch but now I get an error message while compiling the patched source code. Since I don't know if I patched the source code the right way I can't say for sure if this is a problem caused by the patch or me. But what I can say for sure, is that the error is not caused by the source code or the way I try to compile it. I have no problems to compile the unpatched source code.

Here is what I did:
1. Because the header information for kmfoldercachedimap.cpp and kmfoldercachedimap.h in the patch seem to be hard coded for a portage system I removed them and split the patch in two files. Each splitted part included the information for one of two mentioned files to patch.

2. Then I installed the patches with:
# patch ~/kdepim-3.5.8/kmail/kmfoldercachedimap.cpp ~/patch1.diff'
# patch ~/kdepim-3.5.8/kmail/kmfoldercachedimap.h ~/patch2.diff

With patch 1 (for kmfoldercachedimap.cpp) I got the following error messages:

missing header for unified diff at line 1 of patch
patching file /home/niwi/Werkstatt/kdepim-3.5.8/kmail/kmfoldercachedimap.cpp
Hunk #1 succeeded at 189 (offset 9 lines).
Hunk #2 succeeded at 277 (offset 12 lines).
Hunk #3 FAILED at 857.
Hunk #4 succeeded at 1430 (offset 47 lines).
Hunk #5 succeeded at 1531 (offset 92 lines).
patch unexpectedly ends in middle of line
Hunk #6 succeeded at 1843 with fuzz 1 (offset 110 lines).
1 out of 6 hunks FAILED -- saving rejects to file /home/niwi/Werkstatt/kdepim-3.5.8/kmail/kmfoldercachedimap.cpp.rej

I patched hunk #3 manually. The reason for the problem is a missing ")" in line 29,80 of the patch. I checked hunk #5 and 6 and they got patched correctly.

After I installed the patches I did a successful './configure' and started 'make'. 

Here is the error message I finally got:

kmfoldercachedimap.cpp: In member function 'void KMFolderCachedImap::rememberDel                       etion(int)':
kmfoldercachedimap.cpp:495: warning: comparison of unsigned expression >= 0 is a                       lways true
kmfoldercachedimap.cpp: In member function 'void KMFolderCachedImap::serverSyncI                       nternal()':
kmfoldercachedimap.cpp:897: error: could not convert '((KMFolderCachedImap*)this                       )->KMFolderCachedImap::mStatusChangedLocally' to 'bool'
make[3]: *** [kmfoldercachedimap.lo] Error 1
make[3]: Leaving directory `/home/niwi/Werkstatt/kdepim-3.5.8/kmail'
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory `/home/niwi/Werkstatt/kdepim-3.5.8/kmail'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/niwi/Werkstatt/kdepim-3.5.8'
make: *** [all] Error 2


I guess there is a problem with the patch (or me...?).

Any idea what I could do?

Tobias, it looks like you're trying to use the patch against version 3.5.8. The patch was written for version 3.5.7, so it may not work for 3.5.8. It might take a little bit of work to get it to work for the later version if there have been other changes to the code in that area. 

Bonne, thank you for the quick response. Yes, I use KDE 3.5.8. However, 3.5.8 includes KMail 1.9.7, which is the version reported for this bug report. Is it still likely that 3.5.8 reason why I can't compile? Or was the patch made for an earlier version of KMail?

Bonne, your basic idea is sound, although the use of the indeces is dangerous, as you've noted yourself. UIDs should be fine, though, as long as the list is "forgotten" when uid validity changes. Do you think you can adjust your patch that way? If so, I think we can integrate it. Thanks for the contribution, in any case. If you need help, we hang out in #kontact on freenode, or you can post to the kdepim mailing list.

Created attachment 23883 [details]
Revised verson of Bonne's patch working on UIDs

I've revised Bonne's patch for KDE 3.5.9 (kmail 1.9.9) taken from Debian
Unstable. This patch stores UIDs rather than index numbers. It works as
expected (checked by recording protocol traffic) and makes a big difference in
my download time. Thanks Bonne.

Excellent, Jim. I'll have our QA team test the patch on enterprise branch and provided it doesn't cause any regressions, we'll push it into 3.5 branch as well. Thanks a lot for the contribution, this bug is really Free Software at its best, I think. :)

Thanks Jim, I've been overseas away from my dev computer for ages so couldn't get back to this patch. Good to see you picked up my slack, thanks.

I got the same bug Kmail 1.10.3 under KDE 4.1.3.
I use KDE from Debian experimental on a Lenny installation.
This is really annoying as I have more than 30000 mails and the process take something like 30 min. 

KMail 1.11.0 in KDE 4.2 still has this bug.

> KMail 1.11.0 in KDE 4.2 still has this bug.

Looks like the patch was never applied.
Can you change that patch so that it applies against KMail 1.11.0 and send the result to reviewboard.kde.org (kdepim group)? That way, the patch will not get lost.

I have created a review request here: http://reviewboard.kde.org/r/199/ with a new patch which applies against trunk.

Created attachment 31662 [details]
 Patch to only upload status of changed messages (for 4.2)

SVN commit 934167 by tmcguire:

Only update the status of changed messages when syncing cached imap folders,
to speed up folder syncing by a large amount for people with many messages
in their folders.

Thanks to Bonne Eggleston and Jim Hague for the patches!

I slightly modified the patch for config compatibility.

Bonne, please close the review request as submitted.
 
http://reviewboard.kde.org/r/199/
CCMAIL: bonne@ixum.net
BUG: 151916


 M  +42 -11    kmfoldercachedimap.cpp  
 M  +11 -5     kmfoldercachedimap.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=934167

SVN commit 935085 by tmcguire:

Backport r934167 by tmcguire from trunk to the 4.2 branch:

Only update the status of changed messages when syncing cached imap folders,
to speed up folder syncing by a large amount for people with many messages
in their folders.

Thanks to Bonne Eggleston and Jim Hague for the patches!

I slightly modified the patch for config compatibility.

Bonne, please close the review request as submitted.
 
http://reviewboard.kde.org/r/199/
CCMAIL: bonne@ixum.net
CCBUG: 151916



 M  +42 -11    kmfoldercachedimap.cpp  
 M  +11 -5     kmfoldercachedimap.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=935085