From 5194036b423d455a517d06b38fd616a8bbfc4896 Mon Sep 17 00:00:00 2001 From: zachh Date: Tue, 5 Dec 2017 17:42:58 -0800 Subject: Switched CallLogDataSource interface to be Future based. Bug: 34672501 Test: existing PiperOrigin-RevId: 178038086 Change-Id: I1230992ad04bb4415f5a29bd15802d23dff88012 --- java/com/android/dialer/calllog/CallLogModule.java | 4 +- .../calllog/RefreshAnnotatedCallLogWorker.java | 266 +++++++++------------ .../dialer/calllog/database/MutationApplier.java | 29 ++- .../calllog/datasources/CallLogDataSource.java | 10 +- .../dialer/calllog/datasources/DataSources.java | 3 +- .../datasources/contacts/ContactsDataSource.java | 70 ------ .../phonelookup/PhoneLookupDataSource.java | 35 ++- .../systemcalllog/SystemCallLogDataSource.java | 39 ++- .../dialer/calllog/ui/NewCallLogFragment.java | 49 +++- .../common/concurrent/DialerFutureSerializer.java | 98 ++++++++ 10 files changed, 345 insertions(+), 258 deletions(-) delete mode 100644 java/com/android/dialer/calllog/datasources/contacts/ContactsDataSource.java create mode 100644 java/com/android/dialer/common/concurrent/DialerFutureSerializer.java (limited to 'java/com/android') diff --git a/java/com/android/dialer/calllog/CallLogModule.java b/java/com/android/dialer/calllog/CallLogModule.java index 9926cebb9..6c85fd631 100644 --- a/java/com/android/dialer/calllog/CallLogModule.java +++ b/java/com/android/dialer/calllog/CallLogModule.java @@ -18,7 +18,6 @@ package com.android.dialer.calllog; import com.android.dialer.calllog.datasources.CallLogDataSource; import com.android.dialer.calllog.datasources.DataSources; -import com.android.dialer.calllog.datasources.contacts.ContactsDataSource; import com.android.dialer.calllog.datasources.phonelookup.PhoneLookupDataSource; import com.android.dialer.calllog.datasources.systemcalllog.SystemCallLogDataSource; import com.google.common.collect.ImmutableList; @@ -32,11 +31,10 @@ public abstract class CallLogModule { @Provides static DataSources provideCallLogDataSources( SystemCallLogDataSource systemCallLogDataSource, - ContactsDataSource contactsDataSource, PhoneLookupDataSource phoneLookupDataSource) { // System call log must be first, see getDataSourcesExcludingSystemCallLog below. ImmutableList allDataSources = - ImmutableList.of(systemCallLogDataSource, contactsDataSource, phoneLookupDataSource); + ImmutableList.of(systemCallLogDataSource, phoneLookupDataSource); return new DataSources() { @Override public SystemCallLogDataSource getSystemCallLogDataSource() { diff --git a/java/com/android/dialer/calllog/RefreshAnnotatedCallLogWorker.java b/java/com/android/dialer/calllog/RefreshAnnotatedCallLogWorker.java index d9924b23f..de8905db8 100644 --- a/java/com/android/dialer/calllog/RefreshAnnotatedCallLogWorker.java +++ b/java/com/android/dialer/calllog/RefreshAnnotatedCallLogWorker.java @@ -16,204 +16,176 @@ package com.android.dialer.calllog; -import android.annotation.TargetApi; import android.content.Context; -import android.content.OperationApplicationException; import android.content.SharedPreferences; -import android.os.Build; -import android.os.RemoteException; -import android.support.annotation.WorkerThread; import com.android.dialer.calllog.database.CallLogDatabaseComponent; import com.android.dialer.calllog.datasources.CallLogDataSource; import com.android.dialer.calllog.datasources.CallLogMutations; import com.android.dialer.calllog.datasources.DataSources; -import com.android.dialer.common.Assert; import com.android.dialer.common.LogUtil; -import com.android.dialer.common.concurrent.Annotations.UiSerial; +import com.android.dialer.common.concurrent.Annotations.NonUiParallel; +import com.android.dialer.common.concurrent.DialerFutureSerializer; +import com.android.dialer.common.concurrent.DialerFutures; import com.android.dialer.inject.ApplicationContext; import com.android.dialer.storage.Unencrypted; -import com.google.common.util.concurrent.ListenableScheduledFuture; -import com.google.common.util.concurrent.ListeningScheduledExecutorService; +import com.google.common.base.Preconditions; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutorService; import javax.inject.Inject; +import javax.inject.Singleton; /** Brings the annotated call log up to date, if necessary. */ +@Singleton public class RefreshAnnotatedCallLogWorker { - /* - * This is a reasonable time that it might take between related call log writes, that also - * shouldn't slow down single-writes too much. For example, when populating the database using - * the simulator, using this value results in ~6 refresh cycles (on a release build) to write 120 - * call log entries. - */ - private static final long WAIT_MILLIS = 100L; - private final Context appContext; private final DataSources dataSources; private final SharedPreferences sharedPreferences; - private final ListeningScheduledExecutorService listeningScheduledExecutorService; - private ListenableScheduledFuture scheduledFuture; + private final ListeningExecutorService parallelUiListeningExecutorService; + // Used to ensure that only one refresh flow runs at a time. (Note that + // RefreshAnnotatedCallLogWorker is a @Singleton.) + private final DialerFutureSerializer dialerFutureSerializer = new DialerFutureSerializer(); @Inject RefreshAnnotatedCallLogWorker( @ApplicationContext Context appContext, DataSources dataSources, @Unencrypted SharedPreferences sharedPreferences, - @UiSerial ScheduledExecutorService serialUiExecutorService) { + @NonUiParallel ExecutorService parallelUiExecutorService) { this.appContext = appContext; this.dataSources = dataSources; this.sharedPreferences = sharedPreferences; - this.listeningScheduledExecutorService = - MoreExecutors.listeningDecorator(serialUiExecutorService); + + // TODO(zachh): Create and use bindings for ListeningExecutorServices. + this.parallelUiListeningExecutorService = + MoreExecutors.listeningDecorator(parallelUiExecutorService); } /** Checks if the annotated call log is dirty and refreshes it if necessary. */ - public ListenableScheduledFuture refreshWithDirtyCheck() { + public ListenableFuture refreshWithDirtyCheck() { return refresh(true); } /** Refreshes the annotated call log, bypassing dirty checks. */ - public ListenableScheduledFuture refreshWithoutDirtyCheck() { + public ListenableFuture refreshWithoutDirtyCheck() { return refresh(false); } - private ListenableScheduledFuture refresh(boolean checkDirty) { - if (scheduledFuture != null) { - LogUtil.i("RefreshAnnotatedCallLogWorker.refresh", "cancelling waiting task"); - scheduledFuture.cancel(false /* mayInterrupt */); - } - scheduledFuture = - listeningScheduledExecutorService.schedule( - () -> doInBackground(checkDirty), WAIT_MILLIS, TimeUnit.MILLISECONDS); - return scheduledFuture; + private ListenableFuture refresh(boolean checkDirty) { + LogUtil.i("RefreshAnnotatedCallLogWorker.refresh", "submitting serialized refresh request"); + // Note: directExecutor is safe to use here and throughout because all methods are async. + return dialerFutureSerializer.submitAsync( + () -> checkDirtyAndRebuildIfNecessary(appContext, checkDirty), + MoreExecutors.directExecutor()); } - @WorkerThread - private Void doInBackground(boolean checkDirty) - throws RemoteException, OperationApplicationException { - LogUtil.enterBlock("RefreshAnnotatedCallLogWorker.doInBackground"); - - long startTime = System.currentTimeMillis(); - checkDirtyAndRebuildIfNecessary(appContext, checkDirty); - LogUtil.i( - "RefreshAnnotatedCallLogWorker.doInBackground", - "took %dms", - System.currentTimeMillis() - startTime); - return null; - } - - @WorkerThread - private void checkDirtyAndRebuildIfNecessary(Context appContext, boolean checkDirty) - throws RemoteException, OperationApplicationException { - Assert.isWorkerThread(); - - long startTime = System.currentTimeMillis(); - - // Default to true. If the pref doesn't exist, the annotated call log hasn't been created and - // we just skip isDirty checks and force a rebuild. - boolean forceRebuildPrefValue = - sharedPreferences.getBoolean(CallLogFramework.PREF_FORCE_REBUILD, true); - if (forceRebuildPrefValue) { - LogUtil.i( - "RefreshAnnotatedCallLogWorker.checkDirtyAndRebuildIfNecessary", - "annotated call log has been marked dirty or does not exist"); - } - - boolean isDirty = !checkDirty || forceRebuildPrefValue || isDirty(appContext); - - LogUtil.i( - "RefreshAnnotatedCallLogWorker.checkDirtyAndRebuildIfNecessary", - "isDirty took: %dms", - System.currentTimeMillis() - startTime); - if (isDirty) { - startTime = System.currentTimeMillis(); - rebuild(appContext); - LogUtil.i( - "RefreshAnnotatedCallLogWorker.checkDirtyAndRebuildIfNecessary", - "rebuild took: %dms", - System.currentTimeMillis() - startTime); - } + private ListenableFuture checkDirtyAndRebuildIfNecessary( + Context appContext, boolean checkDirty) { + ListenableFuture forceRebuildFuture = + parallelUiListeningExecutorService.submit( + () -> { + LogUtil.i( + "RefreshAnnotatedCallLogWorker.checkDirtyAndRebuildIfNecessary", + "starting refresh flow"); + if (!checkDirty) { + return true; + } + // Default to true. If the pref doesn't exist, the annotated call log hasn't been + // created and we just skip isDirty checks and force a rebuild. + boolean forceRebuildPrefValue = + sharedPreferences.getBoolean(CallLogFramework.PREF_FORCE_REBUILD, true); + if (forceRebuildPrefValue) { + LogUtil.i( + "RefreshAnnotatedCallLogWorker.checkDirtyAndRebuildIfNecessary", + "annotated call log has been marked dirty or does not exist"); + } + return forceRebuildPrefValue; + }); + + // After checking the "force rebuild" shared pref, conditionally call isDirty. + ListenableFuture isDirtyFuture = + Futures.transformAsync( + forceRebuildFuture, + forceRebuild -> + Preconditions.checkNotNull(forceRebuild) + ? Futures.immediateFuture(true) + : isDirty(appContext), + MoreExecutors.directExecutor()); + + // After determining isDirty, conditionally call rebuild. + return Futures.transformAsync( + isDirtyFuture, + isDirty -> + Preconditions.checkNotNull(isDirty) + ? rebuild(appContext) + : Futures.immediateFuture(null), + MoreExecutors.directExecutor()); } - @WorkerThread - private boolean isDirty(Context appContext) { - Assert.isWorkerThread(); - + private ListenableFuture isDirty(Context appContext) { + List> isDirtyFutures = new ArrayList<>(); for (CallLogDataSource dataSource : dataSources.getDataSourcesIncludingSystemCallLog()) { - String dataSourceName = getName(dataSource); - long startTime = System.currentTimeMillis(); - LogUtil.i("RefreshAnnotatedCallLogWorker.isDirty", "running isDirty for %s", dataSourceName); - boolean isDirty = dataSource.isDirty(appContext); - LogUtil.i( - "RefreshAnnotatedCallLogWorker.isDirty", - "%s.isDirty returned %b in %dms", - dataSourceName, - isDirty, - System.currentTimeMillis() - startTime); - if (isDirty) { - return true; - } + isDirtyFutures.add(dataSource.isDirty(appContext)); } - return false; + // Simultaneously invokes isDirty on all data sources, returning as soon as one returns true. + return DialerFutures.firstMatching(isDirtyFutures, Preconditions::checkNotNull, false); } - @TargetApi(Build.VERSION_CODES.M) // Uses try-with-resources - @WorkerThread - private void rebuild(Context appContext) throws RemoteException, OperationApplicationException { - Assert.isWorkerThread(); - + private ListenableFuture rebuild(Context appContext) { CallLogMutations mutations = new CallLogMutations(); - // System call log data source must go first! + // Start by filling the data sources--the system call log data source must go first! CallLogDataSource systemCallLogDataSource = dataSources.getSystemCallLogDataSource(); - String dataSourceName = getName(systemCallLogDataSource); - LogUtil.i("RefreshAnnotatedCallLogWorker.rebuild", "filling %s", dataSourceName); - long startTime = System.currentTimeMillis(); - systemCallLogDataSource.fill(appContext, mutations); - LogUtil.i( - "RefreshAnnotatedCallLogWorker.rebuild", - "%s.fill took: %dms", - dataSourceName, - System.currentTimeMillis() - startTime); + ListenableFuture fillFuture = systemCallLogDataSource.fill(appContext, mutations); + // After the system call log data source is filled, call fill sequentially on each remaining + // data source. This must be done sequentially because mutations are not threadsafe and are + // passed from source to source. for (CallLogDataSource dataSource : dataSources.getDataSourcesExcludingSystemCallLog()) { - dataSourceName = getName(dataSource); - LogUtil.i("RefreshAnnotatedCallLogWorker.rebuild", "filling %s", dataSourceName); - startTime = System.currentTimeMillis(); - dataSource.fill(appContext, mutations); - LogUtil.i( - "CallLogFramework.rebuild", - "%s.fill took: %dms", - dataSourceName, - System.currentTimeMillis() - startTime); + fillFuture = + Futures.transformAsync( + fillFuture, + unused -> dataSource.fill(appContext, mutations), + MoreExecutors.directExecutor()); } - LogUtil.i("RefreshAnnotatedCallLogWorker.rebuild", "applying mutations to database"); - startTime = System.currentTimeMillis(); - CallLogDatabaseComponent.get(appContext) - .mutationApplier() - .applyToDatabase(mutations, appContext); - LogUtil.i( - "RefreshAnnotatedCallLogWorker.rebuild", - "applyToDatabase took: %dms", - System.currentTimeMillis() - startTime); - - for (CallLogDataSource dataSource : dataSources.getDataSourcesIncludingSystemCallLog()) { - dataSourceName = getName(dataSource); - LogUtil.i("RefreshAnnotatedCallLogWorker.rebuild", "onSuccessfulFill'ing %s", dataSourceName); - startTime = System.currentTimeMillis(); - dataSource.onSuccessfulFill(appContext); - LogUtil.i( - "CallLogFramework.rebuild", - "%s.onSuccessfulFill took: %dms", - dataSourceName, - System.currentTimeMillis() - startTime); - } - sharedPreferences.edit().putBoolean(CallLogFramework.PREF_FORCE_REBUILD, false).apply(); - } - private static String getName(CallLogDataSource dataSource) { - return dataSource.getClass().getSimpleName(); + // After all data sources are filled, apply mutations (at this point "fillFuture" is the result + // of filling the last data source). + ListenableFuture applyMutationsFuture = + Futures.transformAsync( + fillFuture, + unused -> + CallLogDatabaseComponent.get(appContext) + .mutationApplier() + .applyToDatabase(mutations, appContext), + MoreExecutors.directExecutor()); + + // After mutations applied, call onSuccessfulFill for each data source (in parallel). + ListenableFuture> onSuccessfulFillFuture = + Futures.transformAsync( + applyMutationsFuture, + unused -> { + List> onSuccessfulFillFutures = new ArrayList<>(); + for (CallLogDataSource dataSource : + dataSources.getDataSourcesIncludingSystemCallLog()) { + onSuccessfulFillFutures.add(dataSource.onSuccessfulFill(appContext)); + } + return Futures.allAsList(onSuccessfulFillFutures); + }, + MoreExecutors.directExecutor()); + + // After onSuccessfulFill is called for every data source, write the shared pref. + return Futures.transform( + onSuccessfulFillFuture, + unused -> { + sharedPreferences.edit().putBoolean(CallLogFramework.PREF_FORCE_REBUILD, false).apply(); + return null; + }, + parallelUiListeningExecutorService); } } diff --git a/java/com/android/dialer/calllog/database/MutationApplier.java b/java/com/android/dialer/calllog/database/MutationApplier.java index 21c8a507d..720daec54 100644 --- a/java/com/android/dialer/calllog/database/MutationApplier.java +++ b/java/com/android/dialer/calllog/database/MutationApplier.java @@ -28,27 +28,44 @@ import com.android.dialer.calllog.database.contract.AnnotatedCallLogContract.Ann import com.android.dialer.calllog.datasources.CallLogMutations; import com.android.dialer.common.Assert; import com.android.dialer.common.LogUtil; +import com.android.dialer.common.concurrent.Annotations.NonUiParallel; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; import java.util.ArrayList; import java.util.Arrays; import java.util.Map.Entry; +import java.util.concurrent.ExecutorService; import javax.inject.Inject; /** Applies {@link CallLogMutations} to the annotated call log. */ public class MutationApplier { + private final ListeningExecutorService executorService; + @Inject - MutationApplier() {} + MutationApplier(@NonUiParallel ExecutorService executorService) { + this.executorService = MoreExecutors.listeningDecorator(executorService); + } /** Applies the provided {@link CallLogMutations} to the annotated call log. */ + public ListenableFuture applyToDatabase(CallLogMutations mutations, Context appContext) { + if (mutations.isEmpty()) { + return Futures.immediateFuture(null); + } + return executorService.submit( + () -> { + applyToDatabaseInternal(mutations, appContext); + return null; + }); + } + @WorkerThread - public void applyToDatabase(CallLogMutations mutations, Context appContext) + private void applyToDatabaseInternal(CallLogMutations mutations, Context appContext) throws RemoteException, OperationApplicationException { Assert.isWorkerThread(); - if (mutations.isEmpty()) { - return; - } - ArrayList operations = new ArrayList<>(); if (!mutations.getInserts().isEmpty()) { diff --git a/java/com/android/dialer/calllog/datasources/CallLogDataSource.java b/java/com/android/dialer/calllog/datasources/CallLogDataSource.java index 3fff3ba53..60654a81a 100644 --- a/java/com/android/dialer/calllog/datasources/CallLogDataSource.java +++ b/java/com/android/dialer/calllog/datasources/CallLogDataSource.java @@ -21,6 +21,7 @@ import android.content.Context; import android.support.annotation.MainThread; import android.support.annotation.WorkerThread; import com.android.dialer.calllog.database.contract.AnnotatedCallLogContract; +import com.google.common.util.concurrent.ListenableFuture; import java.util.List; /** @@ -64,8 +65,7 @@ public interface CallLogDataSource { * * @see CallLogDataSource class doc for complete lifecyle information */ - @WorkerThread - boolean isDirty(Context appContext); + ListenableFuture isDirty(Context appContext); /** * Computes the set of mutations necessary to update the annotated call log with respect to this @@ -76,8 +76,7 @@ public interface CallLogDataSource { * contain inserts from the system call log, and these inserts should be modified by each data * source. */ - @WorkerThread - void fill(Context appContext, CallLogMutations mutations); + ListenableFuture fill(Context appContext, CallLogMutations mutations); /** * Called after database mutations have been applied to all data sources. This is useful for @@ -86,8 +85,7 @@ public interface CallLogDataSource { * * @see CallLogDataSource class doc for complete lifecyle information */ - @WorkerThread - void onSuccessfulFill(Context appContext); + ListenableFuture onSuccessfulFill(Context appContext); /** * Combines raw annotated call log rows into a single coalesced row. diff --git a/java/com/android/dialer/calllog/datasources/DataSources.java b/java/com/android/dialer/calllog/datasources/DataSources.java index 113a9f7b1..9fe6c1db3 100644 --- a/java/com/android/dialer/calllog/datasources/DataSources.java +++ b/java/com/android/dialer/calllog/datasources/DataSources.java @@ -16,13 +16,12 @@ package com.android.dialer.calllog.datasources; -import com.android.dialer.calllog.datasources.systemcalllog.SystemCallLogDataSource; import com.google.common.collect.ImmutableList; /** Immutable lists of data sources used to populate the annotated call log. */ public interface DataSources { - SystemCallLogDataSource getSystemCallLogDataSource(); + CallLogDataSource getSystemCallLogDataSource(); ImmutableList getDataSourcesIncludingSystemCallLog(); diff --git a/java/com/android/dialer/calllog/datasources/contacts/ContactsDataSource.java b/java/com/android/dialer/calllog/datasources/contacts/ContactsDataSource.java deleted file mode 100644 index f0384b09a..000000000 --- a/java/com/android/dialer/calllog/datasources/contacts/ContactsDataSource.java +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright (C) 2017 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License - */ - -package com.android.dialer.calllog.datasources.contacts; - -import android.content.ContentValues; -import android.content.Context; -import android.support.annotation.MainThread; -import android.support.annotation.WorkerThread; -import com.android.dialer.calllog.datasources.CallLogDataSource; -import com.android.dialer.calllog.datasources.CallLogMutations; -import com.android.dialer.common.Assert; -import java.util.List; -import javax.inject.Inject; - -/** Responsible for maintaining the contacts related columns in the annotated call log. */ -public final class ContactsDataSource implements CallLogDataSource { - - @Inject - public ContactsDataSource() {} - - @WorkerThread - @Override - public boolean isDirty(Context appContext) { - Assert.isWorkerThread(); - - // TODO(zachh): Implementation. - return false; - } - - @WorkerThread - @Override - public void fill( - Context appContext, - CallLogMutations mutations) { - Assert.isWorkerThread(); - // TODO(zachh): Implementation. - } - - @Override - public void onSuccessfulFill(Context appContext) { - // TODO(zachh): Implementation. - } - - @Override - public ContentValues coalesce(List individualRowsSortedByTimestampDesc) { - // TODO(zachh): Implementation. - return new ContentValues(); - } - - @MainThread - @Override - public void registerContentObservers( - Context appContext, ContentObserverCallbacks contentObserverCallbacks) { - // TODO(zachh): Guard against missing permissions during callback registration. - } -} diff --git a/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java b/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java index 010cb8541..41eaf2bae 100644 --- a/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java +++ b/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java @@ -29,6 +29,7 @@ import com.android.dialer.calllog.database.contract.AnnotatedCallLogContract.Ann import com.android.dialer.calllog.datasources.CallLogDataSource; import com.android.dialer.calllog.datasources.CallLogMutations; import com.android.dialer.common.LogUtil; +import com.android.dialer.common.concurrent.Annotations.NonUiParallel; import com.android.dialer.phonelookup.PhoneLookup; import com.android.dialer.phonelookup.PhoneLookupInfo; import com.android.dialer.phonelookup.PhoneLookupSelector; @@ -37,6 +38,9 @@ import com.android.dialer.phonenumberproto.DialerPhoneNumberUtil; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; import com.google.i18n.phonenumbers.PhoneNumberUtil; import com.google.protobuf.InvalidProtocolBufferException; import java.util.Arrays; @@ -45,6 +49,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; import javax.inject.Inject; /** @@ -54,15 +59,31 @@ import javax.inject.Inject; public final class PhoneLookupDataSource implements CallLogDataSource { private final PhoneLookup phoneLookup; + private final ListeningExecutorService executorService; @Inject - PhoneLookupDataSource(PhoneLookup phoneLookup) { + PhoneLookupDataSource(PhoneLookup phoneLookup, @NonUiParallel ExecutorService executorService) { this.phoneLookup = phoneLookup; + this.executorService = MoreExecutors.listeningDecorator(executorService); + } + + @Override + public ListenableFuture isDirty(Context appContext) { + return executorService.submit(() -> isDirtyInternal(appContext)); + } + + @Override + public ListenableFuture fill(Context appContext, CallLogMutations mutations) { + return executorService.submit(() -> fillInternal(appContext, mutations)); } - @WorkerThread @Override - public boolean isDirty(Context appContext) { + public ListenableFuture onSuccessfulFill(Context appContext) { + return executorService.submit(this::onSuccessfulFillInternal); + } + + @WorkerThread + private boolean isDirtyInternal(Context appContext) { ImmutableSet uniqueDialerPhoneNumbers = queryDistinctDialerPhoneNumbersFromAnnotatedCallLog(appContext); @@ -102,8 +123,7 @@ public final class PhoneLookupDataSource implements CallLogDataSource { * */ @WorkerThread - @Override - public void fill(Context appContext, CallLogMutations mutations) { + private Void fillInternal(Context appContext, CallLogMutations mutations) { Map> annotatedCallLogIdsByNumber = queryIdAndNumberFromAnnotatedCallLog(appContext); ImmutableMap originalPhoneLookupInfosByNumber = @@ -137,12 +157,13 @@ public final class PhoneLookupDataSource implements CallLogDataSource { } } updateMutations(rowsToUpdate.build(), mutations); + return null; } @WorkerThread - @Override - public void onSuccessfulFill(Context appContext) { + private Void onSuccessfulFillInternal() { // TODO(zachh): Update PhoneLookupHistory. + return null; } @WorkerThread diff --git a/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java b/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java index ef40c308e..dfc768c0a 100644 --- a/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java +++ b/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java @@ -45,15 +45,20 @@ import com.android.dialer.calllog.datasources.util.RowCombiner; import com.android.dialer.calllogutils.PhoneAccountUtils; import com.android.dialer.common.Assert; import com.android.dialer.common.LogUtil; +import com.android.dialer.common.concurrent.Annotations.NonUiParallel; import com.android.dialer.common.concurrent.ThreadUtil; import com.android.dialer.phonenumberproto.DialerPhoneNumberUtil; import com.android.dialer.storage.StorageComponent; import com.android.dialer.theme.R; import com.android.dialer.util.PermissionsUtil; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; import com.google.i18n.phonenumbers.PhoneNumberUtil; import java.util.Arrays; import java.util.List; import java.util.Set; +import java.util.concurrent.ExecutorService; import javax.inject.Inject; /** @@ -66,10 +71,14 @@ public class SystemCallLogDataSource implements CallLogDataSource { @VisibleForTesting static final String PREF_LAST_TIMESTAMP_PROCESSED = "systemCallLogLastTimestampProcessed"; + private final ListeningExecutorService executorService; + @Nullable private Long lastTimestampProcessed; @Inject - public SystemCallLogDataSource() {} + SystemCallLogDataSource(@NonUiParallel ExecutorService executorService) { + this.executorService = MoreExecutors.listeningDecorator(executorService); + } @MainThread @Override @@ -94,9 +103,23 @@ public class SystemCallLogDataSource implements CallLogDataSource { ThreadUtil.getUiThreadHandler(), appContext, contentObserverCallbacks)); } - @WorkerThread @Override - public boolean isDirty(Context appContext) { + public ListenableFuture isDirty(Context appContext) { + return executorService.submit(() -> isDirtyInternal(appContext)); + } + + @Override + public ListenableFuture fill(Context appContext, CallLogMutations mutations) { + return executorService.submit(() -> fillInternal(appContext, mutations)); + } + + @Override + public ListenableFuture onSuccessfulFill(Context appContext) { + return executorService.submit(() -> onSuccessfulFillInternal(appContext)); + } + + @WorkerThread + private boolean isDirtyInternal(Context appContext) { Assert.isWorkerThread(); /* @@ -113,15 +136,14 @@ public class SystemCallLogDataSource implements CallLogDataSource { } @WorkerThread - @Override - public void fill(Context appContext, CallLogMutations mutations) { + private Void fillInternal(Context appContext, CallLogMutations mutations) { Assert.isWorkerThread(); lastTimestampProcessed = null; if (!PermissionsUtil.hasPermission(appContext, permission.READ_CALL_LOG)) { LogUtil.i("SystemCallLogDataSource.fill", "no call log permissions"); - return; + return null; } // This data source should always run first so the mutations should always be empty. @@ -136,11 +158,11 @@ public class SystemCallLogDataSource implements CallLogDataSource { handleInsertsAndUpdates(appContext, mutations, annotatedCallLogIds); handleDeletes(appContext, annotatedCallLogIds, mutations); + return null; } @WorkerThread - @Override - public void onSuccessfulFill(Context appContext) { + private Void onSuccessfulFillInternal(Context appContext) { // If a fill operation was a no-op, lastTimestampProcessed could still be null. if (lastTimestampProcessed != null) { StorageComponent.get(appContext) @@ -149,6 +171,7 @@ public class SystemCallLogDataSource implements CallLogDataSource { .putLong(PREF_LAST_TIMESTAMP_PROCESSED, lastTimestampProcessed) .apply(); } + return null; } @Override diff --git a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java index 6833452c6..a5dccaf69 100644 --- a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java +++ b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java @@ -17,6 +17,7 @@ package com.android.dialer.calllog.ui; import android.database.Cursor; import android.os.Bundle; +import android.support.annotation.Nullable; import android.support.v4.app.Fragment; import android.support.v4.app.LoaderManager.LoaderCallbacks; import android.support.v4.content.Loader; @@ -31,16 +32,26 @@ import com.android.dialer.calllog.CallLogFramework.CallLogUi; import com.android.dialer.calllog.RefreshAnnotatedCallLogWorker; import com.android.dialer.common.LogUtil; import com.android.dialer.common.concurrent.DialerExecutorComponent; +import com.android.dialer.common.concurrent.ThreadUtil; import com.android.dialer.common.concurrent.UiListener; -import com.google.common.util.concurrent.ListenableScheduledFuture; +import com.google.common.util.concurrent.ListenableFuture; /** The "new" call log fragment implementation, which is built on top of the annotated call log. */ public final class NewCallLogFragment extends Fragment implements CallLogUi, LoaderCallbacks { + /* + * This is a reasonable time that it might take between related call log writes, that also + * shouldn't slow down single-writes too much. For example, when populating the database using + * the simulator, using this value results in ~6 refresh cycles (on a release build) to write 120 + * call log entries. + */ + private static final long WAIT_MILLIS = 100L; + private RefreshAnnotatedCallLogWorker refreshAnnotatedCallLogWorker; private UiListener refreshAnnotatedCallLogListener; private RecyclerView recyclerView; + @Nullable private Runnable refreshAnnotatedCallLogRunnable; public NewCallLogFragment() { LogUtil.enterBlock("NewCallLogFragment.NewCallLogFragment"); @@ -81,7 +92,7 @@ public final class NewCallLogFragment extends Fragment callLogFramework.attachUi(this); // TODO(zachh): Consider doing this when fragment becomes visible. - checkAnnotatedCallLogDirtyAndRefreshIfNecessary(); + refreshAnnotatedCallLog(true /* checkDirty */); } @Override @@ -90,6 +101,9 @@ public final class NewCallLogFragment extends Fragment LogUtil.enterBlock("NewCallLogFragment.onPause"); + // This is pending work that we don't actually need to follow through with. + ThreadUtil.getUiThreadHandler().removeCallbacks(refreshAnnotatedCallLogRunnable); + CallLogFramework callLogFramework = CallLogComponent.get(getContext()).callLogFramework(); callLogFramework.detachUi(); } @@ -107,18 +121,35 @@ public final class NewCallLogFragment extends Fragment return view; } - private void checkAnnotatedCallLogDirtyAndRefreshIfNecessary() { - LogUtil.enterBlock("NewCallLogFragment.checkAnnotatedCallLogDirtyAndRefreshIfNecessary"); - ListenableScheduledFuture future = refreshAnnotatedCallLogWorker.refreshWithDirtyCheck(); - refreshAnnotatedCallLogListener.listen(future, unused -> {}, RuntimeException::new); + private void refreshAnnotatedCallLog(boolean checkDirty) { + LogUtil.enterBlock("NewCallLogFragment.refreshAnnotatedCallLog"); + + // If we already scheduled a refresh, cancel it and schedule a new one so that repeated requests + // in quick succession don't result in too much work. For example, if we get 10 requests in + // 10ms, and a complete refresh takes a constant 200ms, the refresh will take 300ms (100ms wait + // and 1 iteration @200ms) instead of 2 seconds (10 iterations @ 200ms) since the work requests + // are serialized in RefreshAnnotatedCallLogWorker. + // + // We might get many requests in quick succession, for example, when the simulator inserts + // hundreds of rows into the system call log, or when the data for a new call is incrementally + // written to different columns as it becomes available. + ThreadUtil.getUiThreadHandler().removeCallbacks(refreshAnnotatedCallLogRunnable); + + refreshAnnotatedCallLogRunnable = + () -> { + ListenableFuture future = + checkDirty + ? refreshAnnotatedCallLogWorker.refreshWithDirtyCheck() + : refreshAnnotatedCallLogWorker.refreshWithoutDirtyCheck(); + refreshAnnotatedCallLogListener.listen(future, unused -> {}, RuntimeException::new); + }; + ThreadUtil.getUiThreadHandler().postDelayed(refreshAnnotatedCallLogRunnable, WAIT_MILLIS); } @Override public void invalidateUi() { LogUtil.enterBlock("NewCallLogFragment.invalidateUi"); - ListenableScheduledFuture future = - refreshAnnotatedCallLogWorker.refreshWithoutDirtyCheck(); - refreshAnnotatedCallLogListener.listen(future, unused -> {}, RuntimeException::new); + refreshAnnotatedCallLog(false /* checkDirty */); } @Override diff --git a/java/com/android/dialer/common/concurrent/DialerFutureSerializer.java b/java/com/android/dialer/common/concurrent/DialerFutureSerializer.java new file mode 100644 index 000000000..2629abbbe --- /dev/null +++ b/java/com/android/dialer/common/concurrent/DialerFutureSerializer.java @@ -0,0 +1,98 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package com.android.dialer.common.concurrent; + +import static com.google.common.util.concurrent.Futures.immediateCancelledFuture; +import static com.google.common.util.concurrent.Futures.immediateFuture; +import static com.google.common.util.concurrent.MoreExecutors.directExecutor; + +import com.google.common.util.concurrent.AsyncCallable; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.SettableFuture; +import java.util.concurrent.Callable; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +/** + * Serializes execution of a set of operations. This class guarantees that a submitted callable will + * not be called before previously submitted callables have completed. + */ +public final class DialerFutureSerializer { + /** This reference acts as a pointer tracking the head of a linked list of ListenableFutures. */ + private final AtomicReference> ref = + new AtomicReference<>(immediateFuture(null)); + + /** Enqueues a task to run when the previous task (if any) completes. */ + public ListenableFuture submit(final Callable callable, Executor executor) { + return submitAsync(() -> immediateFuture(callable.call()), executor); + } + + /** + * Enqueues a task to run when the previous task (if any) completes. + * + *

Cancellation does not propagate from the output future to the future returned from {@code + * callable}, but if the output future is cancelled before {@link AsyncCallable#call()} is + * invoked, {@link AsyncCallable#call()} will not be invoked. + */ + public ListenableFuture submitAsync(final AsyncCallable callable, Executor executor) { + AtomicBoolean wasCancelled = new AtomicBoolean(false); + final AsyncCallable task = + () -> { + if (wasCancelled.get()) { + return immediateCancelledFuture(); + } + return callable.call(); + }; + /* + * Three futures are at play here: + * taskFuture is the future that comes from the callable. + * newFuture is the future we use to track the serialization of our task. + * oldFuture is the previous task's newFuture. + * + * newFuture is guaranteed to only complete once all tasks previously submitted to this instance + * once the futures returned from those submissions have completed. + */ + final SettableFuture newFuture = SettableFuture.create(); + + final ListenableFuture oldFuture = ref.getAndSet(newFuture); + + // Invoke our task once the previous future completes. + final ListenableFuture taskFuture = + Futures.nonCancellationPropagating( + Futures.submitAsync(task, runnable -> oldFuture.addListener(runnable, executor))); + // newFuture's lifetime is determined by taskFuture, unless taskFuture is cancelled, in which + // case it falls back to oldFuture's. This is to ensure that if the future we return is + // cancelled, we don't begin execution of the next task until after oldFuture completes. + taskFuture.addListener( + () -> { + if (taskFuture.isCancelled()) { + // Since the value of oldFuture can only ever be immediateFuture(null) or setFuture of a + // future that eventually came from immediateFuture(null), this doesn't leak throwables + // or completion values. + wasCancelled.set(true); + newFuture.setFuture(oldFuture); + } else { + newFuture.set(null); + } + }, + directExecutor()); + + return taskFuture; + } +} -- cgit v1.2.3