Skip to content
This repository has been archived by the owner on Sep 4, 2021. It is now read-only.

Commit

Permalink
[performance] Schedule resolveAddress immediately
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
amtep committed Oct 17, 2014
1 parent 260da53 commit e2ab061
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 107 deletions.
207 changes: 102 additions & 105 deletions src/seasidecache.cpp
Expand Up @@ -498,7 +498,6 @@ SeasideCache::SeasideCache()
, m_refreshRequired(false)
, m_contactsUpdated(false)
, m_displayOff(false)
, m_activeResolve(0)
{
Q_ASSERT(!instancePtr);
instancePtr = this;
Expand Down Expand Up @@ -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<ResolveData>::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;
Expand Down Expand Up @@ -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<QContactEmailAddress>(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<QContactOnlineAccount>(localFilter, QContactOnlineAccount__FieldAccountPath);
localFilter.setValue(resolve.first);

QContactDetailFilter remoteFilter;
setDetailType<QContactOnlineAccount>(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<QContactSortOrder>());
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()) {
Expand Down Expand Up @@ -2238,13 +2189,9 @@ void SeasideCache::contactsAvailable()
if (contacts.isEmpty())
return;

QSet<QContactDetail::DetailType> queryDetailTypes;
foreach (const QContactDetail::DetailType &typeId, detailTypesHint(fetchHint)) {
queryDetailTypes.insert(typeId);
}
const bool partialFetch = !queryDetailTypes.isEmpty();
QSet<QContactDetail::DetailType> 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
Expand All @@ -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<QPair<QSet<QContactDetail::DetailType>, QList<QContact> > >::iterator it = m_contactsToUpdate.begin(), end = m_contactsToUpdate.end();
Expand Down Expand Up @@ -2330,22 +2277,22 @@ void SeasideCache::applyPendingContactUpdates()
QList<QPair<QSet<QContactDetail::DetailType>, QList<QContact> > >::iterator it = m_contactsToUpdate.begin();

QSet<QContactDetail::DetailType> &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<QContact> &updatedContacts((*it).second);
applyContactUpdates(QList<QContact>() << updatedContacts.takeFirst(), partialFetch, detailTypes);
applyContactUpdates(QList<QContact>() << updatedContacts.takeFirst(), detailTypes);

if (updatedContacts.isEmpty()) {
m_contactsToUpdate.erase(it);
}
}
}

void SeasideCache::applyContactUpdates(const QList<QContact> &contacts, bool partialFetch, const QSet<QContactDetail::DetailType> &queryDetailTypes)
void SeasideCache::applyContactUpdates(const QList<QContact> &contacts, const QSet<QContactDetail::DetailType> &queryDetailTypes)
{
QSet<QString> modifiedGroups;
const bool partialFetch = !queryDetailTypes.isEmpty();

foreach (QContact contact, contacts) {
quint32 iid = internalId(contact);
Expand Down Expand Up @@ -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<QContact> &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<QContactFetchRequest *>(sender());
QSet<QContactDetail::DetailType> queryDetailTypes = detailTypesHint(request->fetchHint()).toSet();
applyContactUpdates(request->contacts(), queryDetailTypes);

// now figure out which address was being resolved and resolve it
QList<ResolveData>::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<QContact> 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)
Expand Down Expand Up @@ -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;
Expand All @@ -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<QContactEmailAddress>(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<QContactOnlineAccount>(localFilter, QContactOnlineAccount__FieldAccountPath);
localFilter.setValue(first);

QContactDetailFilter remoteFilter;
setDetailType<QContactOnlineAccount>(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)
Expand Down
5 changes: 3 additions & 2 deletions src/seasidecache.h
Expand Up @@ -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<QContactId> &contactIds);
void contactsChanged(const QList<QContactId> &contactIds);
Expand Down Expand Up @@ -373,7 +374,7 @@ private slots:
void fetchContacts();
void updateContacts(const QList<QContactId> &contactIds, QList<QContactId> *updateList);
void applyPendingContactUpdates();
void applyContactUpdates(const QList<QContact> &contacts, bool partialFetch, const QSet<QContactDetail::DetailType> &queryDetailTypes);
void applyContactUpdates(const QList<QContact> &contacts, const QSet<QContactDetail::DetailType> &queryDetailTypes);

void resolveUnknownAddresses(const QString &first, const QString &second, CacheItem *item);
bool updateContactIndexing(const QContact &oldContact, const QContact &contact, quint32 iid, const QSet<QContactDetail::DetailType> &queryDetailTypes, CacheItem *item);
Expand Down Expand Up @@ -471,11 +472,11 @@ private slots:
QString compare;
bool requireComplete;
ResolveListener *listener;
QContactFetchRequest *fetchRequest;
};
QList<ResolveData> m_resolveAddresses;
QList<ResolveData> m_unknownResolveAddresses;
QList<ResolveData> m_unknownAddresses;
const ResolveData *m_activeResolve;
QSet<QString> m_resolvedPhoneNumbers;

QElapsedTimer m_timer;
Expand Down

0 comments on commit e2ab061

Please sign in to comment.