Use a plain String instead of CharArrayBuffer.
The code used to use CharArrayBuffer to store the numbers as they are
being processed. However, AbstractCursor.copyToCharArrayBuffer()
actually calls AbstractCursor.getString() and we actually instantiate a
new String when comparing the phone numbers: we might as well call
Cursor.getString() directly and avoid creating two new String objects
each time we need to compare numbers.
Empirically, this seems to reduce the run time of the addGroups()
method.
Bug: 5290401
Change-Id: I397a9e6a14657ce261f8b8c11e607b13083bdef1
diff --git a/src/com/android/contacts/calllog/CallLogGroupBuilder.java b/src/com/android/contacts/calllog/CallLogGroupBuilder.java
index b636842..9829d4a 100644
--- a/src/com/android/contacts/calllog/CallLogGroupBuilder.java
+++ b/src/com/android/contacts/calllog/CallLogGroupBuilder.java
@@ -19,7 +19,6 @@
import com.android.common.widget.GroupingListAdapter;
import com.google.common.annotations.VisibleForTesting;
-import android.database.CharArrayBuffer;
import android.database.Cursor;
import android.provider.CallLog.Calls;
import android.telephony.PhoneNumberUtils;
@@ -34,11 +33,6 @@
public void addGroup(int cursorPosition, int size, boolean expanded);
}
- /** Reusable char array buffer. */
- private CharArrayBuffer mBuffer1 = new CharArrayBuffer(128);
- /** Reusable char array buffer. */
- private CharArrayBuffer mBuffer2 = new CharArrayBuffer(128);
-
/** The object on which the groups are created. */
private final GroupCreator mGroupCreator;
@@ -48,8 +42,8 @@
/**
* Finds all groups of adjacent entries in the call log which should be grouped together and
- * calls {@link CallLogFragment.GroupCreator#addGroup(int, int, boolean)} on
- * {@link #mGroupCreator} for each of them.
+ * calls {@link GroupCreator#addGroup(int, int, boolean)} on {@link #mGroupCreator} for each of
+ * them.
* <p>
* For entries that are not grouped with others, we do not need to create a group of size one.
* <p>
@@ -64,16 +58,14 @@
}
int currentGroupSize = 1;
- // The number of the first entry in the group.
- CharArrayBuffer firstNumber = mBuffer1;
- // The number of the current row in the cursor.
- CharArrayBuffer currentNumber = mBuffer2;
cursor.moveToFirst();
- cursor.copyStringToBuffer(CallLogQuery.NUMBER, firstNumber);
+ // The number of the first entry in the group.
+ String firstNumber = cursor.getString(CallLogQuery.NUMBER);
// This is the type of the first call in the group.
int firstCallType = cursor.getInt(CallLogQuery.CALL_TYPE);
while (cursor.moveToNext()) {
- cursor.copyStringToBuffer(CallLogQuery.NUMBER, currentNumber);
+ // The number of the current row in the cursor.
+ final String currentNumber = cursor.getString(CallLogQuery.NUMBER);
final int callType = cursor.getInt(CallLogQuery.CALL_TYPE);
final boolean sameNumber = equalNumbers(firstNumber, currentNumber);
final boolean shouldGroup;
@@ -105,12 +97,9 @@
}
// Start a new group; it will include at least the current call.
currentGroupSize = 1;
- // The current entry is now the first in the group. For the CharArrayBuffers, we
- // need to swap them.
- firstCallType = callType;
- CharArrayBuffer temp = firstNumber; // Used to swap.
+ // The current entry is now the first in the group.
firstNumber = currentNumber;
- currentNumber = temp;
+ firstCallType = callType;
}
}
// If the last set of calls at the end of the call log was itself a group, create it now.
@@ -131,11 +120,7 @@
}
@VisibleForTesting
- boolean equalNumbers(CharArrayBuffer buffer1, CharArrayBuffer buffer2) {
- // TODO add PhoneNumberUtils.compare(CharSequence, CharSequence) to avoid
- // string allocation
- String number1 = buffer1 == null ? null : new String(buffer1.data, 0, buffer1.sizeCopied);
- String number2 = buffer2 == null ? null : new String(buffer2.data, 0, buffer2.sizeCopied);
+ boolean equalNumbers(String number1, String number2) {
if (PhoneNumberUtils.isUriNumber(number1) || PhoneNumberUtils.isUriNumber(number2)) {
return compareSipAddresses(number1, number2);
} else {
diff --git a/tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java b/tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java
index 2e029cc..24aa428 100644
--- a/tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java
+++ b/tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java
@@ -18,9 +18,6 @@
import static com.google.android.collect.Lists.newArrayList;
-import com.android.contacts.format.FormatUtils;
-
-import android.database.CharArrayBuffer;
import android.database.MatrixCursor;
import android.provider.CallLog.Calls;
import android.test.AndroidTestCase;
@@ -172,40 +169,40 @@
public void testEqualPhoneNumbers() {
// Identical.
- assertTrue(checkEqualNumbers("6505555555", "6505555555"));
- assertTrue(checkEqualNumbers("650 555 5555", "650 555 5555"));
+ assertTrue(mBuilder.equalNumbers("6505555555", "6505555555"));
+ assertTrue(mBuilder.equalNumbers("650 555 5555", "650 555 5555"));
// Formatting.
- assertTrue(checkEqualNumbers("6505555555", "650 555 5555"));
- assertTrue(checkEqualNumbers("6505555555", "(650) 555-5555"));
- assertTrue(checkEqualNumbers("650 555 5555", "(650) 555-5555"));
+ assertTrue(mBuilder.equalNumbers("6505555555", "650 555 5555"));
+ assertTrue(mBuilder.equalNumbers("6505555555", "(650) 555-5555"));
+ assertTrue(mBuilder.equalNumbers("650 555 5555", "(650) 555-5555"));
// Short codes.
- assertTrue(checkEqualNumbers("55555", "55555"));
- assertTrue(checkEqualNumbers("55555", "555 55"));
+ assertTrue(mBuilder.equalNumbers("55555", "55555"));
+ assertTrue(mBuilder.equalNumbers("55555", "555 55"));
// Different numbers.
- assertFalse(checkEqualNumbers("6505555555", "650555555"));
- assertFalse(checkEqualNumbers("6505555555", "6505555551"));
- assertFalse(checkEqualNumbers("650 555 5555", "650 555 555"));
- assertFalse(checkEqualNumbers("650 555 5555", "650 555 5551"));
- assertFalse(checkEqualNumbers("55555", "5555"));
- assertFalse(checkEqualNumbers("55555", "55551"));
+ assertFalse(mBuilder.equalNumbers("6505555555", "650555555"));
+ assertFalse(mBuilder.equalNumbers("6505555555", "6505555551"));
+ assertFalse(mBuilder.equalNumbers("650 555 5555", "650 555 555"));
+ assertFalse(mBuilder.equalNumbers("650 555 5555", "650 555 5551"));
+ assertFalse(mBuilder.equalNumbers("55555", "5555"));
+ assertFalse(mBuilder.equalNumbers("55555", "55551"));
// SIP addresses.
- assertTrue(checkEqualNumbers("6505555555@host.com", "6505555555@host.com"));
- assertTrue(checkEqualNumbers("6505555555@host.com", "6505555555@HOST.COM"));
- assertTrue(checkEqualNumbers("user@host.com", "user@host.com"));
- assertTrue(checkEqualNumbers("user@host.com", "user@HOST.COM"));
- assertFalse(checkEqualNumbers("USER@host.com", "user@host.com"));
- assertFalse(checkEqualNumbers("user@host.com", "user@host1.com"));
+ assertTrue(mBuilder.equalNumbers("6505555555@host.com", "6505555555@host.com"));
+ assertTrue(mBuilder.equalNumbers("6505555555@host.com", "6505555555@HOST.COM"));
+ assertTrue(mBuilder.equalNumbers("user@host.com", "user@host.com"));
+ assertTrue(mBuilder.equalNumbers("user@host.com", "user@HOST.COM"));
+ assertFalse(mBuilder.equalNumbers("USER@host.com", "user@host.com"));
+ assertFalse(mBuilder.equalNumbers("user@host.com", "user@host1.com"));
// SIP address vs phone number.
- assertFalse(checkEqualNumbers("6505555555@host.com", "6505555555"));
- assertFalse(checkEqualNumbers("6505555555", "6505555555@host.com"));
- assertFalse(checkEqualNumbers("user@host.com", "6505555555"));
- assertFalse(checkEqualNumbers("6505555555", "user@host.com"));
+ assertFalse(mBuilder.equalNumbers("6505555555@host.com", "6505555555"));
+ assertFalse(mBuilder.equalNumbers("6505555555", "6505555555@host.com"));
+ assertFalse(mBuilder.equalNumbers("user@host.com", "6505555555"));
+ assertFalse(mBuilder.equalNumbers("6505555555", "user@host.com"));
// Nulls.
- assertTrue(checkEqualNumbers(null, null));
- assertFalse(checkEqualNumbers(null, "6505555555"));
- assertFalse(checkEqualNumbers("6505555555", null));
- assertFalse(checkEqualNumbers(null, "6505555555@host.com"));
- assertFalse(checkEqualNumbers("6505555555@host.com", null));
+ assertTrue(mBuilder.equalNumbers(null, null));
+ assertFalse(mBuilder.equalNumbers(null, "6505555555"));
+ assertFalse(mBuilder.equalNumbers("6505555555", null));
+ assertFalse(mBuilder.equalNumbers(null, "6505555555@host.com"));
+ assertFalse(mBuilder.equalNumbers("6505555555@host.com", null));
}
public void testCompareSipAddresses() {
@@ -227,24 +224,6 @@
assertFalse(mBuilder.compareSipAddresses("6505555555@host.com", null));
}
- /** Calls {@link CallLogGroupBuilder#equalNumbers(CharArrayBuffer, CharArrayBuffer)}. */
- private boolean checkEqualNumbers(String number1, String number2) {
- final CharArrayBuffer buffer1 = stringToCharArrayBuffer(number1);
- final CharArrayBuffer buffer2 = stringToCharArrayBuffer(number2);
- return mBuilder.equalNumbers(buffer1, buffer2);
- }
-
- /** Converts a string into a {@link CharArrayBuffer}, preserving null values. */
- private CharArrayBuffer stringToCharArrayBuffer(String value) {
- if (value == null) {
- return null;
- }
-
- final CharArrayBuffer buffer = new CharArrayBuffer(value.length());
- FormatUtils.copyToCharArrayBuffer(value, buffer);
- return buffer;
- }
-
/** Creates (or recreates) the cursor used to store the call log content for the tests. */
private void createCursor() {
mCursor = new MatrixCursor(CallLogQuery.EXTENDED_PROJECTION);