[Kde-pim] More about KRecursiveFilterProxyModel

[Christian Mollekopf]

On Thursday 29 January 2015 17.09:19 David Faure wrote:
> On Monday 26 January 2015 09:54:11 Christian Mollekopf wrote:
> > On Sunday 25 January 2015 19.55:41 David Faure wrote:
> Right. I found where this came from though: KStatisticsProxyModel's
> dataChanged also emits dataChanged up the hierarchy (because of the case
> where a subtree is closed and the parent folder then shows the aggregate
> totals from the children folder).
> 

Ok.

> So I was right about D, C, B, A, C, B, A, B, A, A, just not about the second
> guilty proxy :) With my KRFPM patch it's down to D, C, B, A because of
> KStaticsProxyModel. How much I wish for Qt5's QVector<int> roles in the
> dataChanged signal....
> > > Apart from reviewing my two RB patches, I'd welcome any input on
> > > whether:
> > > 
> > > * ETM's monitoredCollectionChanged really needs to be emitted so often
> > > (it seems any FetchJob emits it, even if nothing changed?)
> > 
> > What do you mean by *any* fetchjob? Internal fetchjobs?
> > We need emit dataChanged rather frequently because we update i.e.
> > collection statistics through it (whenever an item is added/removed), or
> > the fetch state (if a fetch job is running or not). External fetch jobs
> > should of course not result in a dataChanged signal in the ETM, otherwise
> > we have a bug in akonadi server.
> 
> Yes I mean fetch jobs triggered by the ETM. But during IMAP sync, many
> collections have not changed at all, and yet dataChanged is emitted for
> them, triggering lots of work for nothing.
> 

I guess that is because the fetch state changes (which AFAIK is used for the 
little spinner).

> ETM dataChanged 1
> QModelIndex(0,0,0x22488b0,Akonadi::EntityTreeModel(0x1173940) )  "Personal
> Contacts" 17:01:12 kmail2(29832)/libakonadi
> Akonadi::EntityTreeModelPrivate::monitoredCollectionChanged: collection
> changed 6419
> 
> I know for sure that this collection hasn't changed.
> Can't we avoid calling monitoredCollectionChanged when nothing changed?
> 

I'm sure we could optimize this (i.e. by not exposing the fetchstate if it 
takes less time than n ms), I'm just not sure it's worth it. I think we have 
bigger performance problems than one additional dataChanged signal per 
collection.

> > > * FavoriteCollectionsModel really needs to do so much work on
> > > dataChanged
> > > (Christian, I can't help but wonder if your changes a year ago to this
> > > class, for performance reasons, actually made it worse - I keep seeing
> > > kmail spending time there)
> > 
> > It's of course possible that FavoriteCollectionsModel ends up emitting
> > more
> > dataChanged signals due to this patch. I think at least superfluous
> > selectionChanged signals could be avoided by checking the current
> > selection
> > before selecting it again (QItemSelectionModel::select doesn't do that it
> > seems). Currently we're reselecting the index on every dataChanged signal,
> > and reselecting all indexes on every layoutChanged signals.
> 
> Oh, this is a very good point. I'll try that locally.
> http://www.davidfaure.fr/2015/select-optimisation.diff
> 
> Another thing I've been trying is to comment out the code in the dataChanged
> slot in favoritecollectionsmodel.cpp. I can't see any reason why this would
> ever be useful? Favorite collections are recognized by ID, and an ID
> doesn't change...

I suppose that's true.

> > > * Maybe FavoriteCollectionsModel could sit on top of ETM (or some
> > > intermediate model that just filters for folders) rather than sitting on
> > > top of so many proxies, so that it gets called less often? (the fact
> > > that
> > > any QSFPM turns any source dataChanged into a layoutChanged really
> > > doesn't
> > > help with performance in models sitting on top...)
> > 
> > The stack is (from my notes):
> > 

I forgot EntityMimeTypeFilterModel, which is returned by 
KMKernel::collectionModel() and has the ETM as source.

> > KMKernel::collectionModel() (the ETM)
> > QuotaColorProxyModel (KIdentityProxyModel)
> > KPIM:StatisticsProxyModel (KIdentityProxyModel)
> > FolderTreeWidgetProxyModel (KRecursiveFilterProxyModel)
> > EntityCollectionOrderProxyModel (QSortFilterProxyModel)
> > FavoriteCollectionsModel
> 
> Thanks, that's useful to know.
> 
> > The problem is we probably want the same sorting and filtering, but we
> > could of course just add those models on top of the
> > FavoriteCollectionsModel again.
> 
> Why would we want sorting and filtering in the FavoriteCollectionsModel?
> The model displays N folders of its choice, sorted by the user, doesn't it?

I don't see it doing any sorting?
But other than that we could move it down in the stack. We'd loos quota 
coloring, the rest doesn't really matter as far as I can tell.

Cheers,
Christian

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