On Dec. 8, 2013, 11:28 a.m., Dan Vrátil wrote: > > On the ETM model change, I struggle to believe that Akonadi fetching the > > data then building a model only for us to then scrape the model for the > > data is going to be more efficient than us just loading the data. > > Dan Vrátil wrote: > We don't scrape the model anywhere! The model lives as long as the plugin > lives, which is for the entire application lifetime (in PersonsPluginManager). > > Assuming you use PersonsModel, the full list has been loaded anyway, as > AkonadiAllContacts monitor has been requested, which loads and caches the > whole list in memory. We actually optimize by sharing the contact from the > model between AkonadiAllContacts and AkonadiContact - which means that > requesting AkonadiContact is bit faster, as it does not have to query Akonadi > for the contact, but just looks it up in the model. > > > David Edmundson wrote: > "Assuming you use PersonsModel" > > That's the point - we can't assume the user will have a personsmodel. For > a lot of cases they won't. > > Dan Vrátil wrote: > Ah, I see now. I haven't realized that there's a usecase for the class > outside PersonsModel. > > So what about we have one Akonadi::Monitor in AkonadiAllContacts (like > it's now) and one Akonadi::Monitor in AkonadiDataSource that will monitor all > AkonadiContacts. AkonadiContacts would simply ignore the notification if it's > for a different item. This would still solve the problem with too many > monitors, while not having the model. > > David Edmundson wrote: > To confirm that we're both thinking the same thing: > AkonadiAllContacts is the same > AkonadiContacts doesn't have a monitor itself. > > AkonadiDataSource owns one monitor > AkonadiContact gets the monitor passed as an argument in > createContatMonitor > - runs monitor->setItemMonitored(item, true) in the AkonadiContact > constructor > - runs monitor->setItemMonitored(item, false) in the AkonadiContact > destructor > (these are ref counted so we would never get two AkonadiContacts on the > same ID, so this is safe) > AkonadiContact checks the item changed is the right one before emitting > signals. > >
Yup, exactly :-) - Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114349/#review45341 ----------------------------------------------------------- On Dec. 8, 2013, 3:48 a.m., Dan Vrátil wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/114349/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2013, 3:48 a.m.) > > > Review request for Telepathy. > > > Repository: libkpeople > > > Description > ------- > > Use a single ETM instead of manually fetching all contacts and then > installing a new Akonadi Monitor for each single AkonadiContact. > > ETM (with EntityMimeTypeFilterModel on top) will do the CollectionFetchJobs > and ItemFetchJobs and data handling for us, which only leaves us with > handling of changes, which we do in slots for QAbstractItemModel's > rowsInserted(), rowsRemoved() and dataChanged() signals. This is much better > than having many Akonadi monitors, because for each change notification that > Akonadi server generates, it has to iterate over all registered monitors and > compare the notification against each monitor's filter. With lots of > monitors, this can impact performance of the server. Although the > AkonadiContacts with their Akonadi monitors are created on demand (so there's > none at the beginning), they are not removed (probably cached somewhere in > PersonsModel), so the number of active Akonadi monitors tends to grow - for > instance going through all contacts in PersonViewer will leave you with one > monitor for each contact fed from Akonadi. > > From Akonadi POV, having one properly configured monitor (hidden in ETM) and > working with data via ETM is a better approach. > > > Diffs > ----- > > CMakeLists.txt 889e96d > src/plugins/akonadi/akonadidatasource.h b09edf8 > src/plugins/akonadi/akonadidatasource.cpp 193de78 > > Diff: http://git.reviewboard.kde.org/r/114349/diff/ > > > Testing > ------- > > Going through all contacts in PersonViewer includes all contacts from > Akonadi, and the application uses only one Akonadi Monitor all the time. > > > Thanks, > > Dan Vrátil > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
