[Kde-pim] Review Request: properly honor resource when applying filters to new emails

[Milian Wolff]

> On Sept. 14, 2011, 8:37 p.m., Ingo Klöcker wrote:
> > mailcommon/filtermanager.cpp, line 413
> > <http://git.reviewboard.kde.org/r/102446/diff/2/?file=36131#file36131line413>
> >
> >     It probably doesn't matter that much, but since always the same resource will be used it might make sense to determine the resource just once (before the for-loop) instead of for each filter in the for-loop.

do you really think this introduces performance panalty? If I put it outside the loop I'd have to introduce a temporary (or can't take the param as const&) so I really don't think this is worth it. Imo we can trust the compiler here to do the right thing.

If you are fine with it, I'd like to commit it as-is.


- Milian


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


On Sept. 14, 2011, 12:16 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102446/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2011, 12:16 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Summary
> -------
> 
> This is most probably just a dirty "get it working" patch, but I want to post it here for review. First the problem it solves:
> 
> When starting kmail, it will apply all filters to all new emails without honoring the "apply this filter to incoming messages: from checked accounts only" setting.
> 
> The easiest way to notice is by having a "spamc" or spamassassin (or similar) filter that takes a long time and have that apply to only a very specific account. Then, when a different account gets new email you should see a load spike / spamc processes even though there should be none.
> 
> Imo the whole design of FilterManager::process's function signature is pretty bad. Shouldn't/couldn't the optional stuff be removed and item.parentCollection().resource() be used instead?
> 
> 
> Diffs
> -----
> 
>   mailcommon/filtermanager.h 68e162e 
>   mailcommon/filtermanager.cpp 7d0372e 
> 
> Diff: http://git.reviewboard.kde.org/r/102446/diff
> 
> 
> Testing
> -------
> 
> ran kmail2 for ~1 day. it finally worked like it should
> 
> 
> Thanks,
> 
> Milian
> 
>

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