From e2ab06152995470f375cffa8a501c972ff1b54fa Mon Sep 17 00:00:00 2001 From: Richard Braakman Date: Fri, 17 Oct 2014 14:32:18 +0300 Subject: [PATCH] [performance] Schedule resolveAddress immediately resolveAddress requests are done with one query at a time, in order to correlate the results with the looked-up addresses. Doing them sequentially via UpdateRequest events added a lot of overhead in the form of coordination between threads. This commit creates a dedicated QContactFetchRequest for each resolveAddress call, so that the events for starting them can arrive all together in the backend thread and the events announcing the results can arrive all together in the UI thread. They will still be processed sequentially, but this change cuts out the delay between finishing one request and starting the next. In tests with 10k contacts and a recent contacts list with limit=20, this reduced the resolve time from 1s to 0.3s. --- src/seasidecache.cpp | 207 +++++++++++++++++++++---------------------- src/seasidecache.h | 5 +- 2 files changed, 105 insertions(+), 107 deletions(-) diff --git a/src/seasidecache.cpp b/src/seasidecache.cpp index 8f721f5..1dc6c39 100644 --- a/src/seasidecache.cpp +++ b/src/seasidecache.cpp @@ -498,7 +498,6 @@ SeasideCache::SeasideCache() , m_refreshRequired(false) , m_contactsUpdated(false) , m_displayOff(false) - , m_activeResolve(0) { Q_ASSERT(!instancePtr); instancePtr = this; @@ -664,14 +663,13 @@ void SeasideCache::unregisterResolveListener(ResolveListener *listener) if (!instancePtr) return; - // We might have outstanding resolve requests for this listener - if (instancePtr->m_activeResolve && (instancePtr->m_activeResolve->listener == listener)) { - instancePtr->m_activeResolve = 0; - } - QList::iterator it = instancePtr->m_resolveAddresses.begin(); while (it != instancePtr->m_resolveAddresses.end()) { if (it->listener == listener) { + if (it->fetchRequest) { + it->fetchRequest->cancel(); + delete it->fetchRequest; + } it = instancePtr->m_resolveAddresses.erase(it); } else { ++it; @@ -1456,53 +1454,6 @@ void SeasideCache::startRequest(bool *idleProcessing) } } - // Next priority is handling address resolution requests, - // because these are used to populate the libcommhistory models, - // particularly RecentContactsModel which the user may want to - // interact with immediately. - if (!m_resolveAddresses.isEmpty()) { - if (m_fetchRequest.isActive()) { - requestPending = true; - } else { - const ResolveData &resolve = m_resolveAddresses.first(); - - if (resolve.first.isEmpty()) { - // Search for phone number - m_fetchRequest.setFilter(QContactPhoneNumber::match(resolve.second)); - } else if (resolve.second.isEmpty()) { - // Search for email address - QContactDetailFilter detailFilter; - setDetailType(detailFilter, QContactEmailAddress::FieldEmailAddress); - detailFilter.setMatchFlags(QContactFilter::MatchExactly | QContactFilter::MatchFixedString); // allow case insensitive - detailFilter.setValue(resolve.first); - - m_fetchRequest.setFilter(detailFilter); - } else { - // Search for online account - QContactDetailFilter localFilter; - setDetailType(localFilter, QContactOnlineAccount__FieldAccountPath); - localFilter.setValue(resolve.first); - - QContactDetailFilter remoteFilter; - setDetailType(remoteFilter, QContactOnlineAccount::FieldAccountUri); - remoteFilter.setMatchFlags(QContactFilter::MatchExactly | QContactFilter::MatchFixedString); // allow case insensitive - remoteFilter.setValue(resolve.second); - - m_fetchRequest.setFilter(localFilter & remoteFilter); - } - - // If completion is not required, we need to at least retrieve as much detail - // as the favorites store, so we don't update any favorite with a smaller data subset - m_activeResolve = &resolve; - m_fetchRequest.setFetchHint(resolve.requireComplete ? basicFetchHint() : favoriteFetchHint(m_fetchTypes | m_extraFetchTypes)); - m_fetchRequest.setSorting(QList()); - m_fetchRequest.start(); - - m_fetchProcessedCount = 0; - } - } - - // Then populate the rest of the cache before doing anything else. if (m_keepPopulated && (m_populateProgress != Populated)) { if (m_fetchRequest.isActive()) { @@ -2238,13 +2189,9 @@ void SeasideCache::contactsAvailable() if (contacts.isEmpty()) return; - QSet queryDetailTypes; - foreach (const QContactDetail::DetailType &typeId, detailTypesHint(fetchHint)) { - queryDetailTypes.insert(typeId); - } - const bool partialFetch = !queryDetailTypes.isEmpty(); + QSet queryDetailTypes = detailTypesHint(fetchHint).toSet(); - if (m_populating) { + if (request == &m_fetchRequest && m_populating) { Q_ASSERT(m_populateProgress > Unpopulated && m_populateProgress < Populated); FilterType type(m_populateProgress == FetchFavorites ? FilterFavorites : (m_populateProgress == FetchMetadata ? FilterAll @@ -2258,9 +2205,9 @@ void SeasideCache::contactsAvailable() } requestUpdate(); } else { - if (m_activeResolve || (request == &m_fetchByIdRequest)) { + if (contacts.count() == 1 || request == &m_fetchByIdRequest) { // Process these results immediately - applyContactUpdates(contacts, partialFetch, queryDetailTypes); + applyContactUpdates(contacts, queryDetailTypes); } else { // Add these contacts to the list to be progressively appended QList, QList > >::iterator it = m_contactsToUpdate.begin(), end = m_contactsToUpdate.end(); @@ -2330,12 +2277,11 @@ void SeasideCache::applyPendingContactUpdates() QList, QList > >::iterator it = m_contactsToUpdate.begin(); QSet &detailTypes((*it).first); - const bool partialFetch = !detailTypes.isEmpty(); // Update a single contact at a time; the update can cause numerous QML bindings // to be re-evaluated, so even a single contact update might be a slow operation QList &updatedContacts((*it).second); - applyContactUpdates(QList() << updatedContacts.takeFirst(), partialFetch, detailTypes); + applyContactUpdates(QList() << updatedContacts.takeFirst(), detailTypes); if (updatedContacts.isEmpty()) { m_contactsToUpdate.erase(it); @@ -2343,9 +2289,10 @@ void SeasideCache::applyPendingContactUpdates() } } -void SeasideCache::applyContactUpdates(const QList &contacts, bool partialFetch, const QSet &queryDetailTypes) +void SeasideCache::applyContactUpdates(const QList &contacts, const QSet &queryDetailTypes) { QSet modifiedGroups; + const bool partialFetch = !queryDetailTypes.isEmpty(); foreach (QContact contact, contacts) { quint32 iid = internalId(contact); @@ -2699,56 +2646,71 @@ void SeasideCache::requestStateChanged(QContactAbstractRequest::State state) m_populateProgress = Populated; } m_populating = false; - } else { - // Result of a specific query - if (m_activeResolve) { - if (m_activeResolve->first == QString()) { - // We have now queried this phone number - m_resolvedPhoneNumbers.insert(minimizePhoneNumber(m_activeResolve->second)); - } + } + } - CacheItem *item = 0; - const QList &resolvedContacts(m_fetchRequest.contacts()); - if (!resolvedContacts.isEmpty()) { - if (resolvedContacts.count() == 1) { - item = itemById(apiId(resolvedContacts.first()), false); - } else { - // Lookup the result in our updated indexes - ResolveData data(*m_activeResolve); - if (data.first == QString()) { - item = itemByPhoneNumber(data.second, false); - } else if (data.second == QString()) { - item = itemByEmailAddress(data.first, false); - } else { - item = itemByOnlineAccount(data.first, data.second, false); - } - } + // See if there are any more requests to dispatch + requestUpdate(); +} + +void SeasideCache::addressRequestStateChanged(QContactAbstractRequest::State state) +{ + if (state != QContactAbstractRequest::FinishedState) + return; + + // results are complete, so record them in the cache + QContactFetchRequest *request = static_cast(sender()); + QSet queryDetailTypes = detailTypesHint(request->fetchHint()).toSet(); + applyContactUpdates(request->contacts(), queryDetailTypes); + + // now figure out which address was being resolved and resolve it + QList::iterator it = instancePtr->m_resolveAddresses.begin(); + for ( ; it != instancePtr->m_resolveAddresses.end(); ++it) { + if (request == it->fetchRequest) { + ResolveData data(*it); + if (data.first == QString()) { + // We have now queried this phone number + m_resolvedPhoneNumbers.insert(minimizePhoneNumber(data.second)); + } + + CacheItem *item = 0; + QList resolvedContacts(data.fetchRequest->contacts()); + delete data.fetchRequest; + data.fetchRequest = 0; + + if (!resolvedContacts.isEmpty()) { + if (resolvedContacts.count() == 1) { + item = itemById(apiId(resolvedContacts.first()), false); } else { - // This address is unknown - keep it for later resolution - ResolveData data(*m_activeResolve); + // Lookup the result in our updated indexes if (data.first == QString()) { - // Compare this phone number in minimized form - data.compare = minimizePhoneNumber(data.second); + item = itemByPhoneNumber(data.second, false); } else if (data.second == QString()) { - // Compare this email address in lowercased form - data.compare = data.first.toLower(); + item = itemByEmailAddress(data.first, false); } else { - // Compare this account URI in lowercased form - data.compare = data.second.toLower(); + item = itemByOnlineAccount(data.first, data.second, false); } - - m_unknownAddresses.append(data); } - m_activeResolve->listener->addressResolved(m_activeResolve->first, m_activeResolve->second, item); + } else { + // This address is unknown - keep it for later resolution + if (data.first == QString()) { + // Compare this phone number in minimized form + data.compare = minimizePhoneNumber(data.second); + } else if (data.second == QString()) { + // Compare this email address in lowercased form + data.compare = data.first.toLower(); + } else { + // Compare this account URI in lowercased form + data.compare = data.second.toLower(); + } - m_activeResolve = 0; - m_resolveAddresses.takeFirst(); + m_unknownAddresses.append(data); } + data.listener->addressResolved(data.first, data.second, item); + m_resolveAddresses.erase(it); + return; } } - - // See if there are any more requests to dispatch - requestUpdate(); } void SeasideCache::makePopulated(FilterType filter) @@ -3118,6 +3080,7 @@ void SeasideCache::resolveAddress(ResolveListener *listener, const QString &firs data.second = second; data.requireComplete = requireComplete; data.listener = listener; + data.fetchRequest = 0; // Is this address a known-unknown? bool knownUnknown = false; @@ -3131,11 +3094,45 @@ void SeasideCache::resolveAddress(ResolveListener *listener, const QString &firs if (knownUnknown) { m_unknownResolveAddresses.append(data); + requestUpdate(); } else { + QContactFetchRequest *request = new QContactFetchRequest(this); + request->setManager(manager()); + data.fetchRequest = request; + + if (first.isEmpty()) { + // Search for phone number + request->setFilter(QContactPhoneNumber::match(second)); + } else if (second.isEmpty()) { + // Search for email address + QContactDetailFilter detailFilter; + setDetailType(detailFilter, QContactEmailAddress::FieldEmailAddress); + detailFilter.setMatchFlags(QContactFilter::MatchExactly | QContactFilter::MatchFixedString); // allow case insensitive + detailFilter.setValue(first); + + request->setFilter(detailFilter); + } else { + // Search for online account + QContactDetailFilter localFilter; + setDetailType(localFilter, QContactOnlineAccount__FieldAccountPath); + localFilter.setValue(first); + + QContactDetailFilter remoteFilter; + setDetailType(remoteFilter, QContactOnlineAccount::FieldAccountUri); + remoteFilter.setMatchFlags(QContactFilter::MatchExactly | QContactFilter::MatchFixedString); // allow case insensitive + remoteFilter.setValue(second); + + request->setFilter(localFilter & remoteFilter); + } + + // If completion is not required, we need to at least retrieve as much detail + // as the favorites store, so we don't update any favorite with a smaller data subset + request->setFetchHint(requireComplete ? basicFetchHint() : favoriteFetchHint(m_fetchTypes | m_extraFetchTypes)); + connect(request, SIGNAL(stateChanged(QContactAbstractRequest::State)), + this, SLOT(addressRequestStateChanged(QContactAbstractRequest::State))); m_resolveAddresses.append(data); + request->start(); } - - requestUpdate(); } SeasideCache::CacheItem *SeasideCache::itemMatchingPhoneNumber(const QString &number, const QString &normalized, bool requireComplete) diff --git a/src/seasidecache.h b/src/seasidecache.h index 68deac6..cd50d7b 100644 --- a/src/seasidecache.h +++ b/src/seasidecache.h @@ -342,6 +342,7 @@ private slots: void contactIdsAvailable(); void relationshipsAvailable(); void requestStateChanged(QContactAbstractRequest::State state); + void addressRequestStateChanged(QContactAbstractRequest::State state); void updateContacts(); void contactsAdded(const QList &contactIds); void contactsChanged(const QList &contactIds); @@ -373,7 +374,7 @@ private slots: void fetchContacts(); void updateContacts(const QList &contactIds, QList *updateList); void applyPendingContactUpdates(); - void applyContactUpdates(const QList &contacts, bool partialFetch, const QSet &queryDetailTypes); + void applyContactUpdates(const QList &contacts, const QSet &queryDetailTypes); void resolveUnknownAddresses(const QString &first, const QString &second, CacheItem *item); bool updateContactIndexing(const QContact &oldContact, const QContact &contact, quint32 iid, const QSet &queryDetailTypes, CacheItem *item); @@ -471,11 +472,11 @@ private slots: QString compare; bool requireComplete; ResolveListener *listener; + QContactFetchRequest *fetchRequest; }; QList m_resolveAddresses; QList m_unknownResolveAddresses; QList m_unknownAddresses; - const ResolveData *m_activeResolve; QSet m_resolvedPhoneNumbers; QElapsedTimer m_timer;