From d0d961e19f19704f7967dadf5be61fe6273a1d32 Mon Sep 17 00:00:00 2001 From: Zheng Fu Date: Mon, 1 Jun 2015 14:33:27 -0700 Subject: Performance tuning for contact aggregator2 1. Skip already aggregated raw contacts during aggregation algorithm upgrade. 2. Remove case conversion during email comparion for better performance. 3. Fix bugs in the ContactAggregatorHelper to fix the flaky unit test. Bug:21466686 Bug:19908937 Change-Id: I6f59894a4fdc605fe1b92ac82e2ac9e90561a158 --- .../aggregation/AbstractContactAggregator.java | 79 ++-------- .../contacts/aggregation/ContactAggregator.java | 2 +- .../contacts/aggregation/ContactAggregator2.java | 170 ++++++++++++++++++--- .../aggregation/util/ContactAggregatorHelper.java | 14 +- 4 files changed, 171 insertions(+), 94 deletions(-) (limited to 'src/com/android/providers') diff --git a/src/com/android/providers/contacts/aggregation/AbstractContactAggregator.java b/src/com/android/providers/contacts/aggregation/AbstractContactAggregator.java index 24248b17..0caa861f 100644 --- a/src/com/android/providers/contacts/aggregation/AbstractContactAggregator.java +++ b/src/com/android/providers/contacts/aggregation/AbstractContactAggregator.java @@ -163,7 +163,6 @@ public abstract class AbstractContactAggregator { protected SQLiteStatement mPinnedUpdate; protected SQLiteStatement mContactIdAndMarkAggregatedUpdate; protected SQLiteStatement mContactIdUpdate; - protected SQLiteStatement mMarkAggregatedUpdate; protected SQLiteStatement mContactUpdate; protected SQLiteStatement mContactInsert; protected SQLiteStatement mResetPinnedForRawContact; @@ -360,11 +359,6 @@ public abstract class AbstractContactAggregator { " SET " + RawContacts.CONTACT_ID + "=?" + " WHERE " + RawContacts._ID + "=?"); - mMarkAggregatedUpdate = db.compileStatement( - "UPDATE " + Tables.RAW_CONTACTS + - " SET " + RawContactsColumns.AGGREGATION_NEEDED + "=0" + - " WHERE " + RawContacts._ID + "=?"); - mPresenceContactIdUpdate = db.compileStatement( "UPDATE " + Tables.PRESENCE + " SET " + PresenceColumns.CONTACT_ID + "=?" + @@ -760,7 +754,7 @@ public abstract class AbstractContactAggregator { final String sql = " FROM " + Tables.DATA + " AS d1" + " JOIN " + Tables.DATA + " AS d2" + - " ON lower(d1." + Email.ADDRESS + ")= lower(d2." + Email.ADDRESS + ")" + + " ON d1." + Email.ADDRESS + "= d2." + Email.ADDRESS + " WHERE d1." + DataColumns.MIMETYPE_ID + " = " + emailType + " AND d2." + DataColumns.MIMETYPE_ID + " = " + emailType + " AND d1." + Data.RAW_CONTACT_ID + " IN (" + rawContactIdSet1 + ")" + @@ -898,11 +892,15 @@ public abstract class AbstractContactAggregator { } /** - * Marks the specified raw contact ID as aggregated + * Marks the list of raw contact IDs as aggregated. + * + * @param rawContactIds comma separated raw contact ids */ - protected final void markAggregated(long rawContactId) { - mMarkAggregatedUpdate.bindLong(1, rawContactId); - mMarkAggregatedUpdate.execute(); + protected final void markAggregated(SQLiteDatabase db, String rawContactIds) { + final String sql = "UPDATE " + Tables.RAW_CONTACTS + + " SET " + RawContactsColumns.AGGREGATION_NEEDED + "=0" + + " WHERE " + RawContacts._ID + " in (" + rawContactIds + ")"; + db.execSQL(sql); } /** @@ -1007,33 +1005,6 @@ public abstract class AbstractContactAggregator { } } - protected interface IdentityLookupMatchQuery { - final String TABLE = Tables.DATA + " dataA" - + " JOIN " + Tables.DATA + " dataB" + - " ON (dataA." + Identity.NAMESPACE + "=dataB." + Identity.NAMESPACE + - " AND dataA." + Identity.IDENTITY + "=dataB." + Identity.IDENTITY + ")" - + " JOIN " + Tables.RAW_CONTACTS + - " ON (dataB." + Data.RAW_CONTACT_ID + " = " - + Tables.RAW_CONTACTS + "." + RawContacts._ID + ")"; - - final String SELECTION = "dataA." + Data.RAW_CONTACT_ID + "=?1" - + " AND dataA." + DataColumns.MIMETYPE_ID + "=?2" - + " AND dataA." + Identity.NAMESPACE + " NOT NULL" - + " AND dataA." + Identity.IDENTITY + " NOT NULL" - + " AND dataB." + DataColumns.MIMETYPE_ID + "=?2" - + " AND " + RawContactsColumns.AGGREGATION_NEEDED + "=0" - + " AND " + RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY; - - final String[] COLUMNS = new String[] { - RawContactsColumns.CONCRETE_ID, RawContacts.CONTACT_ID, - RawContactsColumns.ACCOUNT_ID - }; - - int RAW_CONTACT_ID = 0; - int CONTACT_ID = 1; - int ACCOUNT_ID = 2; - } - interface AggregateExceptionQuery { String TABLE = Tables.AGGREGATION_EXCEPTIONS + " JOIN raw_contacts raw_contacts1 " @@ -1064,36 +1035,6 @@ public abstract class AbstractContactAggregator { int AGGREGATION_NEEDED_2 = 8; } - protected interface NameLookupMatchQuery { - String TABLE = Tables.NAME_LOOKUP + " nameA" - + " JOIN " + Tables.NAME_LOOKUP + " nameB" + - " ON (" + "nameA." + NameLookupColumns.NORMALIZED_NAME + "=" - + "nameB." + NameLookupColumns.NORMALIZED_NAME + ")" - + " JOIN " + Tables.RAW_CONTACTS + - " ON (nameB." + NameLookupColumns.RAW_CONTACT_ID + " = " - + Tables.RAW_CONTACTS + "." + RawContacts._ID + ")"; - - String SELECTION = "nameA." + NameLookupColumns.RAW_CONTACT_ID + "=?" - + " AND " + RawContactsColumns.AGGREGATION_NEEDED + "=0" - + " AND " + RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY; - - String[] COLUMNS = new String[] { - RawContacts._ID, - RawContacts.CONTACT_ID, - RawContactsColumns.ACCOUNT_ID, - "nameA." + NameLookupColumns.NORMALIZED_NAME, - "nameA." + NameLookupColumns.NAME_TYPE, - "nameB." + NameLookupColumns.NAME_TYPE, - }; - - int RAW_CONTACT_ID = 0; - int CONTACT_ID = 1; - int ACCOUNT_ID = 2; - int NAME = 3; - int NAME_TYPE_A = 4; - int NAME_TYPE_B = 5; - } - protected interface NameLookupMatchQueryWithParameter { String TABLE = Tables.NAME_LOOKUP + " JOIN " + Tables.RAW_CONTACTS + @@ -1197,7 +1138,7 @@ public abstract class AbstractContactAggregator { protected interface EmailLookupQuery { String TABLE = Tables.DATA + " dataA" + " JOIN " + Tables.DATA + " dataB" + - " ON lower(" + "dataA." + Email.DATA + ")=lower(dataB." + Email.DATA + ")" + " ON dataA." + Email.DATA + "= dataB." + Email.DATA + " JOIN " + Tables.RAW_CONTACTS + " ON (dataB." + Data.RAW_CONTACT_ID + " = " + Tables.RAW_CONTACTS + "." + RawContacts._ID + ")"; diff --git a/src/com/android/providers/contacts/aggregation/ContactAggregator.java b/src/com/android/providers/contacts/aggregation/ContactAggregator.java index 48b893c5..e7fd910f 100644 --- a/src/com/android/providers/contacts/aggregation/ContactAggregator.java +++ b/src/com/android/providers/contacts/aggregation/ContactAggregator.java @@ -187,7 +187,7 @@ public class ContactAggregator extends AbstractContactAggregator { if (contactId == currentContactId) { // Aggregation unchanged - markAggregated(rawContactId); + markAggregated(db, String.valueOf(rawContactId)); if (VERBOSE_LOGGING) { Log.v(TAG, "Aggregation unchanged"); } diff --git a/src/com/android/providers/contacts/aggregation/ContactAggregator2.java b/src/com/android/providers/contacts/aggregation/ContactAggregator2.java index d24b9307..d70e34a8 100644 --- a/src/com/android/providers/contacts/aggregation/ContactAggregator2.java +++ b/src/com/android/providers/contacts/aggregation/ContactAggregator2.java @@ -19,6 +19,9 @@ package com.android.providers.contacts.aggregation; import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; import android.provider.ContactsContract.AggregationExceptions; +import android.provider.ContactsContract.CommonDataKinds.Email; +import android.provider.ContactsContract.CommonDataKinds.Identity; +import android.provider.ContactsContract.CommonDataKinds.Phone; import android.provider.ContactsContract.Contacts.AggregationSuggestions; import android.provider.ContactsContract.Data; import android.provider.ContactsContract.FullNameStyle; @@ -30,6 +33,7 @@ import com.android.providers.contacts.ContactsDatabaseHelper; import com.android.providers.contacts.ContactsDatabaseHelper.DataColumns; import com.android.providers.contacts.ContactsDatabaseHelper.NameLookupColumns; import com.android.providers.contacts.ContactsDatabaseHelper.NameLookupType; +import com.android.providers.contacts.ContactsDatabaseHelper.PhoneLookupColumns; import com.android.providers.contacts.ContactsDatabaseHelper.RawContactsColumns; import com.android.providers.contacts.ContactsDatabaseHelper.Tables; import com.android.providers.contacts.ContactsProvider2; @@ -82,17 +86,6 @@ public class ContactAggregator2 extends AbstractContactAggregator { commonNicknameCache); } - private static class RawContactIdAndAggregationModeQuery { - public static final String TABLE = Tables.RAW_CONTACTS; - - public static final String[] COLUMNS = { RawContacts._ID, RawContacts.AGGREGATION_MODE }; - - public static final String SELECTION = RawContacts.CONTACT_ID + "=?"; - - public static final int _ID = 0; - public static final int AGGREGATION_MODE = 1; - } - /** * Given a specific raw contact, finds all matching raw contacts and re-aggregate them * based on the matching connectivity. @@ -101,6 +94,13 @@ public class ContactAggregator2 extends AbstractContactAggregator { long rawContactId, long accountId, long currentContactId, MatchCandidateList candidates) { + if (!needAggregate(db, rawContactId)) { + if (VERBOSE_LOGGING) { + Log.v(TAG, "Skip rid=" + rawContactId + " which has already been aggregated."); + } + return; + } + if (VERBOSE_LOGGING) { Log.v(TAG, "aggregateContact: rid=" + rawContactId + " cid=" + currentContactId); } @@ -168,7 +168,7 @@ public class ContactAggregator2 extends AbstractContactAggregator { if (VERBOSE_LOGGING) { Log.v(TAG, "Aggregation unchanged"); } - markAggregated(rawContactId); + markAggregated(db, String.valueOf(rawContactId)); } else if (operation == CREATE_NEW_CONTACT) { // create new contact for [rawContactId] if (VERBOSE_LOGGING) { @@ -178,6 +178,7 @@ public class ContactAggregator2 extends AbstractContactAggregator { if (currentContactContentsCount > 0) { updateAggregateData(txContext, currentContactId); } + markAggregated(db, String.valueOf(rawContactId)); } else { // re-aggregate if (VERBOSE_LOGGING) { @@ -189,6 +190,20 @@ public class ContactAggregator2 extends AbstractContactAggregator { } } + private boolean needAggregate(SQLiteDatabase db, long rawContactId) { + final String sql = "SELECT " + RawContacts._ID + " FROM " + Tables.RAW_CONTACTS + + " WHERE " + RawContactsColumns.AGGREGATION_NEEDED + "=1" + + " AND " + RawContacts._ID + "=?"; + + mSelectionArgs1[0] = String.valueOf(rawContactId); + final Cursor cursor = db.rawQuery(sql, mSelectionArgs1); + + try { + return cursor.getCount() != 0; + } finally { + cursor.close(); + } + } /** * Find the set of matching raw contacts for given rawContactId. Add all the raw contact * candidates with matching scores > threshold to RawContactMatchingCandidates. Keep doing @@ -196,15 +211,16 @@ public class ContactAggregator2 extends AbstractContactAggregator { */ private RawContactMatchingCandidates findRawContactMatchingCandidates(SQLiteDatabase db, long rawContactId, MatchCandidateList candidates, RawContactMatcher matcher) { - updateMatchScores(db, rawContactId, candidates, - matcher); + updateMatchScores(db, rawContactId, candidates, matcher); final RawContactMatchingCandidates matchingCandidates = new RawContactMatchingCandidates( matcher.pickBestMatches()); Set newIds = new HashSet<>(); newIds.addAll(matchingCandidates.getRawContactIdSet()); // Keep doing the following until no new raw contact candidate is found. - // TODO: may need to cache the matching score to improve performance. while (!newIds.isEmpty()) { + if (matchingCandidates.getCount() >= AGGREGATION_CONTACT_SIZE_LIMIT) { + return matchingCandidates; + } final Set tmpIdSet = new HashSet<>(); for (long rId : newIds) { final RawContactMatcher rMatcher = new RawContactMatcher(); @@ -348,8 +364,14 @@ public class ContactAggregator2 extends AbstractContactAggregator { } } } - clearSuperPrimarySetting(db, TextUtils.join(",", connectedRawContactIds)); + final String connectedRids = TextUtils.join(",", connectedRawContactIds); + clearSuperPrimarySetting(db, connectedRids); createContactForRawContacts(db, txContext, connectedRawContactIds, contactId); + // re-aggregate + if (VERBOSE_LOGGING) { + Log.v(TAG, "Aggregating rids=" + connectedRawContactIds); + } + markAggregated(db, connectedRids); for (Long cid : cidsNeedToBeUpdated) { long currentRcCount = 0; @@ -504,15 +526,13 @@ public class ContactAggregator2 extends AbstractContactAggregator { long rId = -1; long accountId = -1; if (rawContactId == rawContactId1) { - if (c.getInt(AggregateExceptionQuery.AGGREGATION_NEEDED_2) == 0 - && !c.isNull(AggregateExceptionQuery.RAW_CONTACT_ID2)) { + if (!c.isNull(AggregateExceptionQuery.RAW_CONTACT_ID2)) { rId = c.getLong(AggregateExceptionQuery.RAW_CONTACT_ID2); contactId = c.getLong(AggregateExceptionQuery.CONTACT_ID2); accountId = c.getLong(AggregateExceptionQuery.ACCOUNT_ID2); } } else { - if (c.getInt(AggregateExceptionQuery.AGGREGATION_NEEDED_1) == 0 - && !c.isNull(AggregateExceptionQuery.RAW_CONTACT_ID1)) { + if (!c.isNull(AggregateExceptionQuery.RAW_CONTACT_ID1)) { rId = c.getLong(AggregateExceptionQuery.RAW_CONTACT_ID1); contactId = c.getLong(AggregateExceptionQuery.CONTACT_ID1); accountId = c.getLong(AggregateExceptionQuery.ACCOUNT_ID1); @@ -896,4 +916,114 @@ public class ContactAggregator2 extends AbstractContactAggregator { matchAllCandidates(db, mSb.toString(), candidates, matcher, RawContactMatcher.MATCHING_ALGORITHM_CONSERVATIVE, null); } + + protected interface IdentityLookupMatchQuery { + final String TABLE = Tables.DATA + " dataA" + + " JOIN " + Tables.DATA + " dataB" + + " ON (dataA." + Identity.NAMESPACE + "=dataB." + Identity.NAMESPACE + + " AND dataA." + Identity.IDENTITY + "=dataB." + Identity.IDENTITY + ")" + + " JOIN " + Tables.RAW_CONTACTS + + " ON (dataB." + Data.RAW_CONTACT_ID + " = " + + Tables.RAW_CONTACTS + "." + RawContacts._ID + ")"; + + final String SELECTION = "dataA." + Data.RAW_CONTACT_ID + "=?1" + + " AND dataA." + DataColumns.MIMETYPE_ID + "=?2" + + " AND dataA." + Identity.NAMESPACE + " NOT NULL" + + " AND dataA." + Identity.IDENTITY + " NOT NULL" + + " AND dataB." + DataColumns.MIMETYPE_ID + "=?2" + + " AND " + RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY; + + final String[] COLUMNS = new String[] { + RawContactsColumns.CONCRETE_ID, RawContacts.CONTACT_ID, + RawContactsColumns.ACCOUNT_ID + }; + + int RAW_CONTACT_ID = 0; + int CONTACT_ID = 1; + int ACCOUNT_ID = 2; + } + + protected interface NameLookupMatchQuery { + String TABLE = Tables.NAME_LOOKUP + " nameA" + + " JOIN " + Tables.NAME_LOOKUP + " nameB" + + " ON (" + "nameA." + NameLookupColumns.NORMALIZED_NAME + "=" + + "nameB." + NameLookupColumns.NORMALIZED_NAME + ")" + + " JOIN " + Tables.RAW_CONTACTS + + " ON (nameB." + NameLookupColumns.RAW_CONTACT_ID + " = " + + Tables.RAW_CONTACTS + "." + RawContacts._ID + ")"; + + String SELECTION = "nameA." + NameLookupColumns.RAW_CONTACT_ID + "=?" + + " AND " + RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY; + + String[] COLUMNS = new String[] { + RawContacts._ID, + RawContacts.CONTACT_ID, + RawContactsColumns.ACCOUNT_ID, + "nameA." + NameLookupColumns.NORMALIZED_NAME, + "nameA." + NameLookupColumns.NAME_TYPE, + "nameB." + NameLookupColumns.NAME_TYPE, + }; + + int RAW_CONTACT_ID = 0; + int CONTACT_ID = 1; + int ACCOUNT_ID = 2; + int NAME = 3; + int NAME_TYPE_A = 4; + int NAME_TYPE_B = 5; + } + + protected interface EmailLookupQuery { + String TABLE = Tables.DATA + " dataA" + + " JOIN " + Tables.DATA + " dataB" + + " ON dataA." + Email.DATA + "= dataB." + Email.DATA + + " JOIN " + Tables.RAW_CONTACTS + + " ON (dataB." + Data.RAW_CONTACT_ID + " = " + + Tables.RAW_CONTACTS + "." + RawContacts._ID + ")"; + + String SELECTION = "dataA." + Data.RAW_CONTACT_ID + "=?1" + + " AND dataA." + DataColumns.MIMETYPE_ID + "=?2" + + " AND dataA." + Email.DATA + " NOT NULL" + + " AND dataB." + DataColumns.MIMETYPE_ID + "=?2" + + " AND " + RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY; + + String[] COLUMNS = new String[] { + Tables.RAW_CONTACTS + "." + RawContacts._ID, + RawContacts.CONTACT_ID, + RawContactsColumns.ACCOUNT_ID + }; + + int RAW_CONTACT_ID = 0; + int CONTACT_ID = 1; + int ACCOUNT_ID = 2; + } + + protected interface PhoneLookupQuery { + String TABLE = Tables.PHONE_LOOKUP + " phoneA" + + " JOIN " + Tables.DATA + " dataA" + + " ON (dataA." + Data._ID + "=phoneA." + PhoneLookupColumns.DATA_ID + ")" + + " JOIN " + Tables.PHONE_LOOKUP + " phoneB" + + " ON (phoneA." + PhoneLookupColumns.MIN_MATCH + "=" + + "phoneB." + PhoneLookupColumns.MIN_MATCH + ")" + + " JOIN " + Tables.DATA + " dataB" + + " ON (dataB." + Data._ID + "=phoneB." + PhoneLookupColumns.DATA_ID + ")" + + " JOIN " + Tables.RAW_CONTACTS + + " ON (dataB." + Data.RAW_CONTACT_ID + " = " + + Tables.RAW_CONTACTS + "." + RawContacts._ID + ")"; + + String SELECTION = "dataA." + Data.RAW_CONTACT_ID + "=?" + + " AND PHONE_NUMBERS_EQUAL(dataA." + Phone.NUMBER + ", " + + "dataB." + Phone.NUMBER + ",?)" + + " AND " + RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY; + + String[] COLUMNS = new String[] { + Tables.RAW_CONTACTS + "." + RawContacts._ID, + RawContacts.CONTACT_ID, + RawContactsColumns.ACCOUNT_ID + }; + + int RAW_CONTACT_ID = 0; + int CONTACT_ID = 1; + int ACCOUNT_ID = 2; + } + } diff --git a/src/com/android/providers/contacts/aggregation/util/ContactAggregatorHelper.java b/src/com/android/providers/contacts/aggregation/util/ContactAggregatorHelper.java index 983cd609..71eaceaa 100644 --- a/src/com/android/providers/contacts/aggregation/util/ContactAggregatorHelper.java +++ b/src/com/android/providers/contacts/aggregation/util/ContactAggregatorHelper.java @@ -58,21 +58,27 @@ public class ContactAggregatorHelper { } index++; } - final Set mergedSet = new HashSet<>(); + connectedRawContactSets.clear(); for (Long accountId : accounts.keySet()) { final Set s = accounts.get(accountId); if (s.size() > 1) { for (Integer i : s) { final Set rIdSet = rawContactIds.get(i); - if (rIdSet != null) { + if (rIdSet != null && !rIdSet.isEmpty()) { connectedRawContactSets.add(rawContactIds.get(i)); rawContactIds.remove(i); } } - } else { + } + } + + final Set mergedSet = new HashSet<>(); + for (Long accountId : accounts.keySet()) { + final Set s = accounts.get(accountId); + if (s.size() == 1) { Set ids = rawContactIds.get(Iterables.getOnlyElement(s)); - if (ids != null) { + if (ids != null && !ids.isEmpty()) { mergedSet.addAll(ids); } } -- cgit v1.2.3