[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] Review Request: Optimize file access in maildir
From:       "Kevin Krammer" <kevin.krammer () gmx ! at>
Date:       2012-02-18 15:24:49
Message-ID: 20120218152449.2508.27827 () vidsolbach ! de
[Download RAW message or body]


-----------------------------------------------------------
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@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/


[prev in list] [next in list] [prev in thread] [next in thread]