Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4885a025 authored by Flavio Lerda's avatar Flavio Lerda
Browse files

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
parent 5c357cdc

src/com/android/contacts/calllog/CallLogGroupBuilder.java

+9 −24
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ package com.android.contacts.calllog; 
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 class CallLogGroupBuilder { 
        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 @@ public class CallLogGroupBuilder { 


    /**

     * 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 @@ public class CallLogGroupBuilder { 
        }



        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 @@ public class CallLogGroupBuilder { 
                }

                // 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 @@ public class CallLogGroupBuilder { 
    }



    @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 {

tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java

+28 −49
Original line number Diff line number Diff line
@@ -18,9 +18,6 @@ package com.android.contacts.calllog; 


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 class CallLogGroupBuilderTest extends AndroidTestCase { 


    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 @@ public class CallLogGroupBuilderTest extends AndroidTestCase { 
        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);