From a44febc6e294bf805f82a3a16f05e23b89676e37 Mon Sep 17 00:00:00 2001 From: Danning Chen Date: Mon, 9 Mar 2020 17:19:44 -0700 Subject: [PATCH] Run PeopleService.onUserUnlocked() on worker thread instead of main thread This change is targeting on reducing the latency to unlock a user. It includes these changes: (1) In DataManager.onUserUnlocked(), execute the listeners registration code on a worker thread. (2) Merge the usage stats service query and data persistence to one thread. (3) Adjust the query events max age from 1 day to 5 mins because we already persist those events in PeopleService. No need to re-query those events. With this change, PeopleService.onUserUnlocked() execution is reduced from ~200 ms to <1ms. And listeners are set up after conversations being loaded from the disk. This ensures at the events query time (such as UsageStatsService query), all the conversations are already available in People Service to avoid missing some events (events without an associated conversation are dropped). Change-Id: I5248afa2771588a2434c335867de4e2d1a2eb6f3 Test: atest com.android.server.people.data.DataManagerTest Test: atest com.android.server.people.data.ConversationStoreTest Test: atest com.android.server.people.data.PackageDataTest Bug: 150733302 --- .../server/people/data/ConversationStore.java | 34 ++- .../server/people/data/DataManager.java | 204 ++++++++++-------- .../server/people/data/EventStore.java | 26 +-- .../server/people/data/PackageData.java | 2 + .../android/server/people/data/UserData.java | 16 +- .../people/data/ConversationStoreTest.java | 5 +- .../server/people/data/DataManagerTest.java | 5 + 7 files changed, 156 insertions(+), 136 deletions(-) diff --git a/services/people/java/com/android/server/people/data/ConversationStore.java b/services/people/java/com/android/server/people/data/ConversationStore.java index 8481e5b916a64..28e3d4b5b7448 100644 --- a/services/people/java/com/android/server/people/data/ConversationStore.java +++ b/services/people/java/com/android/server/people/data/ConversationStore.java @@ -89,25 +89,21 @@ class ConversationStore { * Loads conversations from disk to memory in a background thread. This should be called * after the device powers on and the user has been unlocked. */ - @MainThread - void loadConversationsFromDisk() { - mScheduledExecutorService.execute(() -> { - synchronized (this) { - ConversationInfosProtoDiskReadWriter conversationInfosProtoDiskReadWriter = - getConversationInfosProtoDiskReadWriter(); - if (conversationInfosProtoDiskReadWriter == null) { - return; - } - List conversationsOnDisk = - conversationInfosProtoDiskReadWriter.read(CONVERSATIONS_FILE_NAME); - if (conversationsOnDisk == null) { - return; - } - for (ConversationInfo conversationInfo : conversationsOnDisk) { - updateConversationsInMemory(conversationInfo); - } - } - }); + @WorkerThread + synchronized void loadConversationsFromDisk() { + ConversationInfosProtoDiskReadWriter conversationInfosProtoDiskReadWriter = + getConversationInfosProtoDiskReadWriter(); + if (conversationInfosProtoDiskReadWriter == null) { + return; + } + List conversationsOnDisk = + conversationInfosProtoDiskReadWriter.read(CONVERSATIONS_FILE_NAME); + if (conversationsOnDisk == null) { + return; + } + for (ConversationInfo conversationInfo : conversationsOnDisk) { + updateConversationsInMemory(conversationInfo); + } } /** diff --git a/services/people/java/com/android/server/people/data/DataManager.java b/services/people/java/com/android/server/people/data/DataManager.java index 4e0afc5253832..7085f96dd31a6 100644 --- a/services/people/java/com/android/server/people/data/DataManager.java +++ b/services/people/java/com/android/server/people/data/DataManager.java @@ -28,6 +28,7 @@ import android.app.prediction.AppTarget; import android.app.prediction.AppTargetEvent; import android.content.BroadcastReceiver; import android.content.ComponentName; +import android.content.ContentResolver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; @@ -87,13 +88,13 @@ public class DataManager { private static final String TAG = "DataManager"; - private static final long QUERY_EVENTS_MAX_AGE_MS = DateUtils.DAY_IN_MILLIS; + private static final long QUERY_EVENTS_MAX_AGE_MS = 5L * DateUtils.MINUTE_IN_MILLIS; private static final long USAGE_STATS_QUERY_INTERVAL_SEC = 120L; private final Context mContext; private final Injector mInjector; - private final ScheduledExecutorService mUsageStatsQueryExecutor; - private final ScheduledExecutorService mDiskReadWriterExecutor; + private final ScheduledExecutorService mScheduledExecutor; + private final Object mLock = new Object(); private final SparseArray mUserDataArray = new SparseArray<>(); private final SparseArray mBroadcastReceivers = new SparseArray<>(); @@ -118,8 +119,7 @@ public class DataManager { DataManager(Context context, Injector injector) { mContext = context; mInjector = injector; - mUsageStatsQueryExecutor = mInjector.createScheduledExecutor(); - mDiskReadWriterExecutor = mInjector.createScheduledExecutor(); + mScheduledExecutor = mInjector.createScheduledExecutor(); } /** Initialization. Called when the system services are up running. */ @@ -138,103 +138,56 @@ public class DataManager { /** This method is called when a user is unlocked. */ public void onUserUnlocked(int userId) { - UserData userData = mUserDataArray.get(userId); - if (userData == null) { - userData = new UserData(userId, mDiskReadWriterExecutor); - mUserDataArray.put(userId, userData); + synchronized (mLock) { + UserData userData = mUserDataArray.get(userId); + if (userData == null) { + userData = new UserData(userId, mScheduledExecutor); + mUserDataArray.put(userId, userData); + } + userData.setUserUnlocked(); } - userData.setUserUnlocked(); - updateDefaultDialer(userData); - updateDefaultSmsApp(userData); - - ScheduledFuture scheduledFuture = mUsageStatsQueryExecutor.scheduleAtFixedRate( - new UsageStatsQueryRunnable(userId), 1L, USAGE_STATS_QUERY_INTERVAL_SEC, - TimeUnit.SECONDS); - mUsageStatsQueryFutures.put(userId, scheduledFuture); - - IntentFilter intentFilter = new IntentFilter(); - intentFilter.addAction(TelecomManager.ACTION_DEFAULT_DIALER_CHANGED); - intentFilter.addAction(SmsApplication.ACTION_DEFAULT_SMS_PACKAGE_CHANGED_INTERNAL); - BroadcastReceiver broadcastReceiver = new PerUserBroadcastReceiver(userId); - mBroadcastReceivers.put(userId, broadcastReceiver); - mContext.registerReceiverAsUser( - broadcastReceiver, UserHandle.of(userId), intentFilter, null, null); - - ContentObserver contactsContentObserver = new ContactsContentObserver( - BackgroundThread.getHandler()); - mContactsContentObservers.put(userId, contactsContentObserver); - mContext.getContentResolver().registerContentObserver( - Contacts.CONTENT_URI, /* notifyForDescendants= */ true, - contactsContentObserver, userId); - - NotificationListener notificationListener = new NotificationListener(); - mNotificationListeners.put(userId, notificationListener); - try { - notificationListener.registerAsSystemService(mContext, - new ComponentName(mContext, getClass()), userId); - } catch (RemoteException e) { - // Should never occur for local calls. - } - - PackageMonitor packageMonitor = new PerUserPackageMonitor(); - packageMonitor.register(mContext, null, UserHandle.of(userId), true); - mPackageMonitors.put(userId, packageMonitor); - - if (userId == UserHandle.USER_SYSTEM) { - // The call log and MMS/SMS messages are shared across user profiles. So only need to - // register the content observers once for the primary user. - // TODO: Register observers after the conversations and events being loaded from disk. - mCallLogContentObserver = new CallLogContentObserver(BackgroundThread.getHandler()); - mContext.getContentResolver().registerContentObserver( - CallLog.CONTENT_URI, /* notifyForDescendants= */ true, - mCallLogContentObserver, UserHandle.USER_SYSTEM); - - mMmsSmsContentObserver = new MmsSmsContentObserver(BackgroundThread.getHandler()); - mContext.getContentResolver().registerContentObserver( - MmsSms.CONTENT_URI, /* notifyForDescendants= */ false, - mMmsSmsContentObserver, UserHandle.USER_SYSTEM); - } - - DataMaintenanceService.scheduleJob(mContext, userId); + mScheduledExecutor.execute(() -> setupUser(userId)); } /** This method is called when a user is stopping. */ public void onUserStopping(int userId) { - if (mUserDataArray.indexOfKey(userId) >= 0) { - mUserDataArray.get(userId).setUserStopped(); - } - if (mUsageStatsQueryFutures.indexOfKey(userId) >= 0) { - mUsageStatsQueryFutures.get(userId).cancel(true); - } - if (mBroadcastReceivers.indexOfKey(userId) >= 0) { - mContext.unregisterReceiver(mBroadcastReceivers.get(userId)); - } - if (mContactsContentObservers.indexOfKey(userId) >= 0) { - mContext.getContentResolver().unregisterContentObserver( - mContactsContentObservers.get(userId)); - } - if (mNotificationListeners.indexOfKey(userId) >= 0) { - try { - mNotificationListeners.get(userId).unregisterAsSystemService(); - } catch (RemoteException e) { - // Should never occur for local calls. + synchronized (mLock) { + ContentResolver contentResolver = mContext.getContentResolver(); + if (mUserDataArray.indexOfKey(userId) >= 0) { + mUserDataArray.get(userId).setUserStopped(); } - } - if (mPackageMonitors.indexOfKey(userId) >= 0) { - mPackageMonitors.get(userId).unregister(); - } - if (userId == UserHandle.USER_SYSTEM) { - if (mCallLogContentObserver != null) { - mContext.getContentResolver().unregisterContentObserver(mCallLogContentObserver); - mCallLogContentObserver = null; + if (mUsageStatsQueryFutures.indexOfKey(userId) >= 0) { + mUsageStatsQueryFutures.get(userId).cancel(true); } - if (mMmsSmsContentObserver != null) { - mContext.getContentResolver().unregisterContentObserver(mMmsSmsContentObserver); - mCallLogContentObserver = null; + if (mBroadcastReceivers.indexOfKey(userId) >= 0) { + mContext.unregisterReceiver(mBroadcastReceivers.get(userId)); + } + if (mContactsContentObservers.indexOfKey(userId) >= 0) { + contentResolver.unregisterContentObserver(mContactsContentObservers.get(userId)); + } + if (mNotificationListeners.indexOfKey(userId) >= 0) { + try { + mNotificationListeners.get(userId).unregisterAsSystemService(); + } catch (RemoteException e) { + // Should never occur for local calls. + } + } + if (mPackageMonitors.indexOfKey(userId) >= 0) { + mPackageMonitors.get(userId).unregister(); + } + if (userId == UserHandle.USER_SYSTEM) { + if (mCallLogContentObserver != null) { + contentResolver.unregisterContentObserver(mCallLogContentObserver); + mCallLogContentObserver = null; + } + if (mMmsSmsContentObserver != null) { + contentResolver.unregisterContentObserver(mMmsSmsContentObserver); + mCallLogContentObserver = null; + } } - } - DataMaintenanceService.cancelJob(mContext, userId); + DataMaintenanceService.cancelJob(mContext, userId); + } } /** @@ -288,6 +241,9 @@ public class DataManager { return; } UserData userData = getUnlockedUserData(appTarget.getUser().getIdentifier()); + if (userData == null) { + return; + } PackageData packageData = userData.getOrCreatePackageData(appTarget.getPackageName()); String mimeType = intentFilter != null ? intentFilter.getDataType(0) : null; @Event.EventType int eventType = mimeTypeToShareEventType(mimeType); @@ -353,6 +309,68 @@ public class DataManager { userData.restore(payload); } + private void setupUser(@UserIdInt int userId) { + synchronized (mLock) { + UserData userData = getUnlockedUserData(userId); + if (userData == null) { + return; + } + userData.loadUserData(); + + updateDefaultDialer(userData); + updateDefaultSmsApp(userData); + + ScheduledFuture scheduledFuture = mScheduledExecutor.scheduleAtFixedRate( + new UsageStatsQueryRunnable(userId), 1L, USAGE_STATS_QUERY_INTERVAL_SEC, + TimeUnit.SECONDS); + mUsageStatsQueryFutures.put(userId, scheduledFuture); + + IntentFilter intentFilter = new IntentFilter(); + intentFilter.addAction(TelecomManager.ACTION_DEFAULT_DIALER_CHANGED); + intentFilter.addAction(SmsApplication.ACTION_DEFAULT_SMS_PACKAGE_CHANGED_INTERNAL); + BroadcastReceiver broadcastReceiver = new PerUserBroadcastReceiver(userId); + mBroadcastReceivers.put(userId, broadcastReceiver); + mContext.registerReceiverAsUser( + broadcastReceiver, UserHandle.of(userId), intentFilter, null, null); + + ContentObserver contactsContentObserver = new ContactsContentObserver( + BackgroundThread.getHandler()); + mContactsContentObservers.put(userId, contactsContentObserver); + mContext.getContentResolver().registerContentObserver( + Contacts.CONTENT_URI, /* notifyForDescendants= */ true, + contactsContentObserver, userId); + + NotificationListener notificationListener = new NotificationListener(); + mNotificationListeners.put(userId, notificationListener); + try { + notificationListener.registerAsSystemService(mContext, + new ComponentName(mContext, getClass()), userId); + } catch (RemoteException e) { + // Should never occur for local calls. + } + + PackageMonitor packageMonitor = new PerUserPackageMonitor(); + packageMonitor.register(mContext, null, UserHandle.of(userId), true); + mPackageMonitors.put(userId, packageMonitor); + + if (userId == UserHandle.USER_SYSTEM) { + // The call log and MMS/SMS messages are shared across user profiles. So only need + // to register the content observers once for the primary user. + mCallLogContentObserver = new CallLogContentObserver(BackgroundThread.getHandler()); + mContext.getContentResolver().registerContentObserver( + CallLog.CONTENT_URI, /* notifyForDescendants= */ true, + mCallLogContentObserver, UserHandle.USER_SYSTEM); + + mMmsSmsContentObserver = new MmsSmsContentObserver(BackgroundThread.getHandler()); + mContext.getContentResolver().registerContentObserver( + MmsSms.CONTENT_URI, /* notifyForDescendants= */ false, + mMmsSmsContentObserver, UserHandle.USER_SYSTEM); + } + + DataMaintenanceService.scheduleJob(mContext, userId); + } + } + private int mimeTypeToShareEventType(String mimeType) { if (mimeType.startsWith("text/")) { return Event.TYPE_SHARE_TEXT; diff --git a/services/people/java/com/android/server/people/data/EventStore.java b/services/people/java/com/android/server/people/data/EventStore.java index 00d4241fc5f73..9cf84c94f0f70 100644 --- a/services/people/java/com/android/server/people/data/EventStore.java +++ b/services/people/java/com/android/server/people/data/EventStore.java @@ -17,9 +17,9 @@ package com.android.server.people.data; import android.annotation.IntDef; -import android.annotation.MainThread; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.WorkerThread; import android.net.Uri; import android.util.ArrayMap; @@ -90,20 +90,16 @@ class EventStore { * Loads existing {@link EventHistoryImpl}s from disk. This should be called when device powers * on and user is unlocked. */ - @MainThread - void loadFromDisk() { - mScheduledExecutorService.execute(() -> { - synchronized (this) { - for (@EventCategory int category = 0; category < mEventsCategoryDirs.size(); - category++) { - File categoryDir = mEventsCategoryDirs.get(category); - Map existingEventHistoriesImpl = - EventHistoryImpl.eventHistoriesImplFromDisk(categoryDir, - mScheduledExecutorService); - mEventHistoryMaps.get(category).putAll(existingEventHistoriesImpl); - } - } - }); + @WorkerThread + synchronized void loadFromDisk() { + for (@EventCategory int category = 0; category < mEventsCategoryDirs.size(); + category++) { + File categoryDir = mEventsCategoryDirs.get(category); + Map existingEventHistoriesImpl = + EventHistoryImpl.eventHistoriesImplFromDisk(categoryDir, + mScheduledExecutorService); + mEventHistoryMaps.get(category).putAll(existingEventHistoriesImpl); + } } /** diff --git a/services/people/java/com/android/server/people/data/PackageData.java b/services/people/java/com/android/server/people/data/PackageData.java index 3e4c992ce70b3..28837d5bf1480 100644 --- a/services/people/java/com/android/server/people/data/PackageData.java +++ b/services/people/java/com/android/server/people/data/PackageData.java @@ -25,6 +25,7 @@ import static com.android.server.people.data.EventStore.CATEGORY_SMS; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UserIdInt; +import android.annotation.WorkerThread; import android.content.LocusId; import android.os.FileUtils; import android.text.TextUtils; @@ -77,6 +78,7 @@ public class PackageData { * Returns a map of package directory names as keys and their associated {@link PackageData}. * This should be called when device is powered on and unlocked. */ + @WorkerThread @NonNull static Map packagesDataFromDisk(@UserIdInt int userId, @NonNull Predicate isDefaultDialerPredicate, diff --git a/services/people/java/com/android/server/people/data/UserData.java b/services/people/java/com/android/server/people/data/UserData.java index ed8c595ab42a4..429d5b7193ceb 100644 --- a/services/people/java/com/android/server/people/data/UserData.java +++ b/services/people/java/com/android/server/people/data/UserData.java @@ -19,6 +19,7 @@ package com.android.server.people.data; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UserIdInt; +import android.annotation.WorkerThread; import android.os.Environment; import android.text.TextUtils; import android.util.ArrayMap; @@ -75,12 +76,6 @@ class UserData { void setUserUnlocked() { mIsUnlocked = true; - - // Ensures per user root directory for people data is present, and attempt to load - // data from disk. - mPerUserPeopleDataDir.mkdirs(); - mPackageDataMap.putAll(PackageData.packagesDataFromDisk(mUserId, this::isDefaultDialer, - this::isDefaultSmsApp, mScheduledExecutorService, mPerUserPeopleDataDir)); } void setUserStopped() { @@ -91,6 +86,15 @@ class UserData { return mIsUnlocked; } + @WorkerThread + void loadUserData() { + mPerUserPeopleDataDir.mkdir(); + Map packageDataMap = PackageData.packagesDataFromDisk( + mUserId, this::isDefaultDialer, this::isDefaultSmsApp, mScheduledExecutorService, + mPerUserPeopleDataDir); + mPackageDataMap.putAll(packageDataMap); + } + /** * Gets the {@link PackageData} for the specified {@code packageName} if exists; otherwise * creates a new instance and returns it. diff --git a/services/tests/servicestests/src/com/android/server/people/data/ConversationStoreTest.java b/services/tests/servicestests/src/com/android/server/people/data/ConversationStoreTest.java index 4e63237a6091f..e10cbbfe78adf 100644 --- a/services/tests/servicestests/src/com/android/server/people/data/ConversationStoreTest.java +++ b/services/tests/servicestests/src/com/android/server/people/data/ConversationStoreTest.java @@ -273,9 +273,8 @@ public final class ConversationStoreTest { // Ensure that futures were cancelled and the immediate flush occurred. assertEquals(0, mMockScheduledExecutorService.getFutures().size()); - // Expect to see 2 executes: loadConversationFromDisk and saveConversationsToDisk. - // loadConversationFromDisk gets called each time we call #resetConversationStore(). - assertEquals(2, mMockScheduledExecutorService.getExecutes().size()); + // Expect to see 1 execute: saveConversationsToDisk. + assertEquals(1, mMockScheduledExecutorService.getExecutes().size()); resetConversationStore(); ConversationInfo out1 = mConversationStore.getConversation(SHORTCUT_ID); diff --git a/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java b/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java index a4d63ac8a5649..51996047a74bc 100644 --- a/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java @@ -183,6 +183,11 @@ public final class DataManagerTest { when(mExecutorService.scheduleAtFixedRate(any(Runnable.class), anyLong(), anyLong(), any( TimeUnit.class))).thenReturn(mScheduledFuture); + doAnswer(ans -> { + Runnable runnable = (Runnable) ans.getArguments()[0]; + runnable.run(); + return null; + }).when(mExecutorService).execute(any(Runnable.class)); when(mUserManager.getEnabledProfiles(USER_ID_PRIMARY)) .thenReturn(Arrays.asList(