[Kde-pim] Review Request 114505: ItemFetchJob: Add setCollectItems

[Kevin Krammer]

On Dec. 16, 2013, 7:06 p.m., Vishesh Handa wrote:
> > I would suggest we look at this in a slightly more flexible context, i.e. delivery option flags.
> > e.g.:
> > 
> > enum DeliveryOption {
> >   None = 0x0,            // no item delivery, not really useful
> >   ItemGetter = 0x1,      // items available through items()
> >   ImmediateSignal = 0x2, // emitted via signal upon reception
> >   BulkSignal = 0x4,      // emitted via signal in bulk (collected and emitted delayed via timer)
> >   Default = ItemGetter | BulkSignal
> > };
> > Q_FLAGS(DeliverOptions, DeliveryOption);
> 
> Dan Vrátil wrote:
>     I like this idea. It allows nice optimizations for various use-cases. 
>     
>     Maybe we could consider changing the default behaviour for KF5 to use BulkSignal?
> 
> Christian Mollekopf wrote:
>     What's the advantage of BulkSignal vs ItemGetter? All items are in memory at once anyways, no?
>     Having the option to chose between the efficient signal and the convenient accessor probably makes sense for KF5 though.

There might not be any difference depending on the amount of time the job takes to complete.
ItemGetter requires that all received items are stored inside the job object, while BulkSignal only requires to store as many items as are received within the timer interval (which is 100ms AFAIK).
So if the job completes within the timer interval then memory wise there will be no difference between ItemGetter and BulkSignal. If the job takes longer than that, then BulkSignal will require less memory since the job object only ever holds a portion of the items.

I am not sure we can easily change the default in KF5 due to this being a behavioral change and difficult to detect.
The most convenient way is ItemGetter, since it only requires connecting to result(KJob*) which one would do anyway. So it makes a good default IMHO.


- Kevin


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


On Dec. 16, 2013, 6:31 p.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114505/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2013, 6:31 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Repository: kdepimlibs
> 
> 
> Description
> -------
> 
>     This allows us to control if the items being fetched are internally
>     collected in order to be accessed later via the items() function.
>     
>     We want to disable this in the case of email indexing where every
>     item in every collection is loaded into memory along with its entire
>     payload. Collecting everything results in very high memory usage.
> 
> 
> Diffs
> -----
> 
>   akonadi/itemfetchjob.cpp 93168c3 
>   akonadi/itemfetchjob.h f9c52a2 
> 
> Diff: http://git.reviewboard.kde.org/r/114505/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

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