[Kde-pim] [Differential] [Commented On] D1636: Implement client-side threading cache

[dfaure (David Faure)]

dfaure added a comment.


  Yeah... I can't find the courage to dig into the actual threading code, sorry.
  
  Are there unit tests for this logic BTW?

INLINE COMMENTS

> dvratil wrote in model.cpp:762
> The in-memory cache gets dynamically updated when new emails arrive and are threaded by the model (or simply when you open a folder there's no cache for yet). When new folder is opened, Model::setStorageModel() is called and that's when we save() the in-memory cache of the old folder threading into a file, and then load() threading cache of the newly-selected folder.

I see. Can you add a "// save cache for old folder" comment, for the next dumb reader like me? :-)

Also.... what about the first time? Shouldn't save only be done if there *is* an old folder?

> model.cpp:763
> +    d->mThreadingCache.save();
> +    d->mThreadingCache.load(d->mStorageModel->id(), d->mAggregation);
> +

What if the user chose no threading, i.e. flat list?
Does the cache bring anything to the picture in that case, or just overhead?
AFAICS it's just overhead, writing out the full list of all items with a null parent into the cache. So I think the cache should be skipped if no threading is happening.

I use flat list in many folders, I'd like this opportunity while it's fresh in your head to ask if anything can be done to make that fast too :-)

> threadingcache.cpp:97
> +    QDataStream stream(&cacheFile);
> +    CacheHeader cacheHeader= {};
> +    stream >> cacheHeader;

Missing space before =

> threadingcache.cpp:125
> +        if (stream.status() != QDataStream::Ok) {
> +            // Suspect corrupted cache
> +            cacheFile.close();

No qCDebug in this case? All other return paths have one...

REPOSITORY
  rMESSAGELIB PIM: Message Library

REVISION DETAIL
  https://phabricator.kde.org/D1636

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dvratil, mlaurent, vkrause, dfaure
Cc: kde-pim, dvasin, winterz, smartins, vkrause, mlaurent, knauss, dvratil
_______________________________________________
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/