[Kde-pim] Review Request: Optimize file access in maildir

[Kevin Krammer]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103947/#review10721
-----------------------------------------------------------



resources/maildir/libmaildir/keycache.h
<http://git.reviewboard.kde.org/r/103947/#comment8725>

    I know this is already inconsistent in the maildir.h file, but I think KDEPIM coding style favors
    QString &dir
    over
    QString& dir
    



resources/maildir/libmaildir/keycache.h
<http://git.reviewboard.kde.org/r/103947/#comment8728>

    Hmm, I think KDEPIM coding style suggest using members in this form:
    mNewKeys



resources/maildir/libmaildir/keycache.cpp
<http://git.reviewboard.kde.org/r/103947/#comment8726>

    Won't newKeys.size() always be 0 here?
    dir is not in m_newKeys, so m_newKeys[dir] will create and return an empty set, no?
    
    In case this is true, could this not just be written as
    m_newKeys[dir] = listNew(dir)
    



resources/maildir/libmaildir/keycache.cpp
<http://git.reviewboard.kde.org/r/103947/#comment8727>

    What about
    return m_curKeys.value( dir ).contains( key );



resources/maildir/maildirresource.cpp
<http://git.reviewboard.kde.org/r/103947/#comment8724>

    I know that it not something you have changed, but this should probably be
    setFlag( flag )
    i.e. spaces around function argument


- Kevin Krammer


On Feb. 18, 2012, 11:36 a.m., Andras Mantia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103947/
> -----------------------------------------------------------
> 
> (Updated Feb. 18, 2012, 11:36 a.m.)
> 
> 
> Review request for KDEPIM and Kevin Krammer.
> 
> 
> Description
> -------
> 
> This patch reduces the number of file stats (QFile::exist) in the maildir library by caching the content of the cur/new directory listing and using QMap lookup to search for file existance.
> The patch also optimizes the mapping between collections and Maildir objects in the resource.
> 
> 
> Diffs
> -----
> 
>   resources/maildir/libmaildir/CMakeLists.txt b8cfb92 
>   resources/maildir/libmaildir/keycache.h PRE-CREATION 
>   resources/maildir/libmaildir/keycache.cpp PRE-CREATION 
>   resources/maildir/libmaildir/maildir.h b038f02 
>   resources/maildir/libmaildir/maildir.cpp d61ac0b 
>   resources/maildir/maildirresource.h 233732c 
>   resources/maildir/maildirresource.cpp b5aa3e2 
>   resources/maildir/retrieveitemsjob.h dfc7095 
>   resources/maildir/retrieveitemsjob.cpp 97f22b3 
> 
> Diff: http://git.reviewboard.kde.org/r/103947/diff/
> 
> 
> Testing
> -------
> 
> 1) make test in libmaildir passes
> 2) I run a variant of the patch for quite some time (~2 months) without problem. The only difference was using a QStringList instead of QMap for caching, which make the code even slower. :)
> 3) You can see the poor man's profiling with the QTime: with a 18K folder subsequent synching (when nothing changes, just click on the folder in KMail that triggers a sync), is around 1100ms. With the patch it is around 670ms. With a 30K folder it was above 2000ms, now it is below 1200ms. This is the time for "ENTRIESPROCESSED", the actual amount of time needed to compare the items in the database with the files on the disk.
> 
> Here are the full numbers (3 runs for each folder, numbers in ms):
> 
> 18K folder, original code:
> LOCALLISTDONE AT 1856 
> ENTRIESPROCESSED AT 1101 
> 
> LOCALLISTDONE AT 1879 
> ENTRIESPROCESSED AT 1098 
> 
> LOCALLISTDONE AT 1809 
> ENTRIESPROCESSED AT 1097 
> 
> 
> 18K - with patch:
> LOCALLISTDONE AT 1923 
> ENTRIESPROCESSED AT 675 
> 
> LOCALLISTDONE AT 2002 
> ENTRIESPROCESSED AT 674 
> 
> LOCALLISTDONE AT 2045 
> ENTRIESPROCESSED AT 669 
> 
> Net gain is 300-400ms.
> 
> 32K-original:
> LOCALLISTDONE AT 3627 
> ENTRIESPROCESSED AT 2041 
> 
> LOCALLISTDONE AT 3826 
> ENTRIESPROCESSED AT 2009 
> 
> LOCALLISTDONE AT 3698 
> ENTRIESPROCESSED AT 2024 
> 
> 
> 32K -with patch:
> LOCALLISTDONE AT 3447 
> ENTRIESPROCESSED AT 1304 
> 
> LOCALLISTDONE AT 3728 
> ENTRIESPROCESSED AT 1216 
> 
> 
> LOCALLISTDONE AT 3723 
> ENTRIESPROCESSED AT 1207 
> 
> Net gain is 900-1000ms.
> 
> 
> Thanks,
> 
> Andras Mantia
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/