From e548aa3b150e8fdda429cf4e89a1cbcede3bdc90 Mon Sep 17 00:00:00 2001 From: Matt Vogt Date: Wed, 20 Nov 2013 13:58:12 +1000 Subject: [PATCH] [libcontacts] Do not use iterators during coalescing This practice is possibly affected by iterator invalidation in some way I can't locate. In any case, index-based operations are not going to make any performance difference. --- src/seasideimport.cpp | 80 ++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 43 deletions(-) diff --git a/src/seasideimport.cpp b/src/seasideimport.cpp index 13e64e7..7418a1c 100644 --- a/src/seasideimport.cpp +++ b/src/seasideimport.cpp @@ -319,15 +319,13 @@ QList SeasideImport::buildImportContacts(const QList QList importedContacts(importer.contacts()); - QList::iterator> obsoleteContacts; - - QHash::iterator> importGuids; - QHash::iterator> importNames; - QHash::iterator> importLabels; + QHash importGuids; + QHash importNames; + QHash importLabels; // Merge any duplicates in the import list - QList::iterator it = importedContacts.begin(), end = importedContacts.end(); - for ( ; it != end; ++it) { + QList::iterator it = importedContacts.begin(); + while (it != importedContacts.end()) { QContact &contact(*it); const QString guid = contact.detail().guid(); @@ -335,50 +333,47 @@ QList SeasideImport::buildImportContacts(const QList const QString name = contactNameString(contact); const QString label = contact.detail().label(); - QContact *previous = 0; - QHash::iterator>::const_iterator git = importGuids.find(guid); + int previousIndex = -1; + QHash::const_iterator git = importGuids.find(guid); if (git != importGuids.end()) { - QContact &guidContact(*(git.value())); - previous = &guidContact; + previousIndex = git.value(); } else { - QHash::iterator>::const_iterator nit = importNames.find(name); + QHash::const_iterator nit = importNames.find(name); if (nit != importNames.end()) { - QContact &nameContact(*(nit.value())); - previous = &nameContact; + previousIndex = nit.value(); } else { // Only if name is empty, use displayLabel - probably SIM import if (emptyName) { - QHash::iterator>::const_iterator lit = importLabels.find(label); + QHash::const_iterator lit = importLabels.find(label); if (lit != importLabels.end()) { - QContact &labelContact(*(lit.value())); - previous = &labelContact; + previousIndex = lit.value(); } } } } - if (previous) { + if (previousIndex != -1) { // Combine these duplicate contacts - mergeIntoExistingContact(previous, contact); - obsoleteContacts.prepend(it); + QContact &previous(importedContacts[previousIndex]); + mergeIntoExistingContact(&previous, contact); + + it = importedContacts.erase(it); } else { + const int index = it - importedContacts.begin(); if (!guid.isEmpty()) { - importGuids.insert(guid, it); + importGuids.insert(guid, index); } if (!emptyName) { - importNames.insert(name, it); + importNames.insert(name, index); } else if (!label.isEmpty()) { - importLabels.insert(label, it); + importLabels.insert(label, index); // Modify this contact to have the label as a nickname setNickname(contact, label); } - } - } - // Remove contacts whose details were merged into other contacts - foreach (QList::iterator it, obsoleteContacts) { - importedContacts.erase(it); + ++it; + } } // Find all names and GUIDs for local contacts that might match these contacts @@ -411,10 +406,10 @@ QList SeasideImport::buildImportContacts(const QList } // Find any imported contacts that match contacts we already have - QMap::iterator> existingIds; - QList::iterator> duplicates; + QMap existingIds; - for (it = importedContacts.begin(), end = importedContacts.end(); it != end; ++it) { + it = importedContacts.begin(); + while (it != importedContacts.end()) { const QString guid = (*it).detail().guid(); QContactId existingId; @@ -450,23 +445,23 @@ QList SeasideImport::buildImportContacts(const QList } if (existing) { - QMap::iterator>::iterator eit = existingIds.find(existingId); + QMap::iterator eit = existingIds.find(existingId); if (eit == existingIds.end()) { - existingIds.insert(existingId, it); + existingIds.insert(existingId, (it - importedContacts.begin())); + + ++it; } else { // Combine these contacts with matching names - QList::iterator cit(*eit); - mergeIntoExistingContact(&*cit, *it); + QContact &previous(importedContacts[*eit]); + mergeIntoExistingContact(&previous, *it); - duplicates.append(it); + it = importedContacts.erase(it); } + } else { + ++it; } } - // Remove any duplicates we identified - while (!duplicates.isEmpty()) - importedContacts.erase(duplicates.takeLast()); - int existingCount(existingIds.count()); if (existingCount > 0) { // Retrieve all the contacts that we have matches for @@ -485,11 +480,10 @@ QList SeasideImport::buildImportContacts(const QList QSet unmodifiedContacts; foreach (const QContact &contact, mgr->contacts(idFilter & localContactFilter(), QList(), basicFetchHint())) { - QMap::iterator>::const_iterator it = existingIds.find(contact.id()); + QMap::const_iterator it = existingIds.find(contact.id()); if (it != existingIds.end()) { // Update the existing version of the contact with any new details - QList::iterator cit(*it); - QContact &importContact(*cit); + QContact &importContact(importedContacts[*it]); bool modified = updateExistingContact(&importContact, contact); if (modified) { modifiedContacts.insert(importContact.id());