[Kde-pim] Review Request 116815: Optimize the address parsing by caching already parsed addresses

[Kevin Krammer]

> On March 15, 2014, 7:59 p.m., Kevin Krammer wrote:
> > A couple of things:
> > - the access to the hash is not thread safe
> > - the code doesn't seem to add anything to the cache
> > - it doesn't have any limitation on used memory, see QCache
> 
> Mark Gaiser wrote:
>     ehh, oops. I had two patches (one with an iterator based search and one with .exists() and .value()) The non iterator based one inserts as well, this one - mistakenly - doesn't. Seems like i mixed up the two.
>     Fixing that now.
>     
>     Regarding thread safety. I guess that depends if threads are going to be used on this class. Thus far it "doesn't look like it" but i hardly know for sure. For reading it should be fine since the documentation statues: "In addition, they are thread-safe in situations where they are used as read-only containers by all threads used to access them."
>     
>     Don't know for QCache either. I guess that depends if the process using this class is going to be alive as long as akonadi is running or if it's only going to be alive when it actually needs indexing. I really hope you know the answer there :)

This is the mail item serializer, it is uses by all applications that use KMime::Message items, e.g. all email resources.
My guess is that the seralizer loader does not load the plugin for each item individually so it will be around as long as the process is running.

I also think that the same instance is shared between threads, which is usually not a problem since the serializers are stateless. It is true that your current patch has only read access, but I assume that the real code will write into the hash at some point, right?


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116815/#review53009
-----------------------------------------------------------


On March 15, 2014, 5:02 p.m., Mark Gaiser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116815/
> -----------------------------------------------------------
> 
> (Updated March 15, 2014, 5:02 p.m.)
> 
> 
> Review request for KDEPIM, Dan Vrátil and Vishesh Handa.
> 
> 
> Repository: kdepim-runtime
> 
> 
> Description
> -------
> 
> Profiling mail indexing showed that parsing the address line was taking up most time. Further checking showed that each address is parsed, also if it was already parsed. So i simply added a cache in between which means that addresses are only parsed if not parsed before. This should speed up indexing significantly.
> 
> In my benchmarks indexing was twice as fast as opposed to before this patch. But on git master i'm getting wildly different results. Still faster, but just about 20%.
> 
> 
> Diffs
> -----
> 
>   plugins/akonadi_serializer_mail.cpp a6ac4b8 
> 
> Diff: https://git.reviewboard.kde.org/r/116815/diff/
> 
> 
> Testing
> -------
> 
> All tests pass just fine.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

_______________________________________________
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/