From 73093a477dbf4422cccf19233e691c5d2fdf21f3 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Tue, 3 Oct 2017 15:27:56 -0700 Subject: [PATCH] Backup OS service now processes unlock off the main thread We now perform the slow work of unlocking the primary user on the background handler thread rather than on the main thread. This is expected to shave 150ms+ off of our total unlock latency on some products. Bug: 63389800 Test: manual Change-Id: I70be032a64de3270476615fd9dde1b427e19aa91 --- .../server/backup/BackupManagerService.java | 78 ++++++++++--------- .../backup/BackupManagerServiceInterface.java | 2 + .../RefactoredBackupManagerService.java | 78 ++++++++++--------- .../com/android/server/backup/Trampoline.java | 7 ++ 4 files changed, 95 insertions(+), 70 deletions(-) diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java index f4dbb5a9b0b26..eabe21fed9df8 100644 --- a/services/backup/java/com/android/server/backup/BackupManagerService.java +++ b/services/backup/java/com/android/server/backup/BackupManagerService.java @@ -401,45 +401,53 @@ public class BackupManagerService implements BackupManagerServiceInterface { @Override public void onUnlockUser(int userId) { if (userId == UserHandle.USER_SYSTEM) { - Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup init"); - sInstance.initialize(userId); - Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); - - // Migrate legacy setting - Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup migrate"); - if (!backupSettingMigrated(userId)) { - if (DEBUG) { - Slog.i(TAG, "Backup enable apparently not migrated"); - } - final ContentResolver r = sInstance.mContext.getContentResolver(); - final int enableState = Settings.Secure.getIntForUser(r, - Settings.Secure.BACKUP_ENABLED, -1, userId); - if (enableState >= 0) { - if (DEBUG) { - Slog.i(TAG, "Migrating enable state " + (enableState != 0)); - } - writeBackupEnableState(enableState != 0, userId); - Settings.Secure.putStringForUser(r, - Settings.Secure.BACKUP_ENABLED, null, userId); - } else { - if (DEBUG) { - Slog.i(TAG, "Backup not yet configured; retaining null enable state"); - } - } - } - Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); - - Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup enable"); - try { - sInstance.setBackupEnabled(readBackupEnableState(userId)); - } catch (RemoteException e) { - // can't happen; it's a local object - } - Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); + sInstance.unlockSystemUser(); } } } + // Called through the trampoline from onUnlockUser(), then we buck the work + // off to the background thread to keep the unlock time down. + public void unlockSystemUser() { + mBackupHandler.post(() -> { + Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup init"); + sInstance.initialize(UserHandle.USER_SYSTEM); + Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); + + // Migrate legacy setting + Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup migrate"); + if (!backupSettingMigrated(UserHandle.USER_SYSTEM)) { + if (DEBUG) { + Slog.i(TAG, "Backup enable apparently not migrated"); + } + final ContentResolver r = sInstance.mContext.getContentResolver(); + final int enableState = Settings.Secure.getIntForUser(r, + Settings.Secure.BACKUP_ENABLED, -1, UserHandle.USER_SYSTEM); + if (enableState >= 0) { + if (DEBUG) { + Slog.i(TAG, "Migrating enable state " + (enableState != 0)); + } + writeBackupEnableState(enableState != 0, UserHandle.USER_SYSTEM); + Settings.Secure.putStringForUser(r, + Settings.Secure.BACKUP_ENABLED, null, UserHandle.USER_SYSTEM); + } else { + if (DEBUG) { + Slog.i(TAG, "Backup not yet configured; retaining null enable state"); + } + } + } + Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); + + Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup enable"); + try { + sInstance.setBackupEnabled(readBackupEnableState(UserHandle.USER_SYSTEM)); + } catch (RemoteException e) { + // can't happen; it's a local object + } + Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); + }); + } + class ProvisionedObserver extends ContentObserver { public ProvisionedObserver(Handler handler) { super(handler); diff --git a/services/backup/java/com/android/server/backup/BackupManagerServiceInterface.java b/services/backup/java/com/android/server/backup/BackupManagerServiceInterface.java index 5dfa6302093e6..041f9ed5d0314 100644 --- a/services/backup/java/com/android/server/backup/BackupManagerServiceInterface.java +++ b/services/backup/java/com/android/server/backup/BackupManagerServiceInterface.java @@ -39,6 +39,8 @@ import java.io.PrintWriter; */ public interface BackupManagerServiceInterface { + void unlockSystemUser(); + // Utility: build a new random integer token int generateRandomIntegerToken(); diff --git a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java index fe4902f8bf9da..f298065903a80 100644 --- a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java @@ -548,45 +548,53 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter @Override public void onUnlockUser(int userId) { if (userId == UserHandle.USER_SYSTEM) { - Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup init"); - sInstance.initialize(userId); - Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); - - // Migrate legacy setting - Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup migrate"); - if (!backupSettingMigrated(userId)) { - if (DEBUG) { - Slog.i(TAG, "Backup enable apparently not migrated"); - } - final ContentResolver r = sInstance.mContext.getContentResolver(); - final int enableState = Settings.Secure.getIntForUser(r, - Settings.Secure.BACKUP_ENABLED, -1, userId); - if (enableState >= 0) { - if (DEBUG) { - Slog.i(TAG, "Migrating enable state " + (enableState != 0)); - } - writeBackupEnableState(enableState != 0, userId); - Settings.Secure.putStringForUser(r, - Settings.Secure.BACKUP_ENABLED, null, userId); - } else { - if (DEBUG) { - Slog.i(TAG, "Backup not yet configured; retaining null enable state"); - } - } - } - Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); - - Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup enable"); - try { - sInstance.setBackupEnabled(readBackupEnableState(userId)); - } catch (RemoteException e) { - // can't happen; it's a local object - } - Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); + sInstance.unlockSystemUser(); } } } + // Called through the trampoline from onUnlockUser(), then we buck the work + // off to the background thread to keep the unlock time down. + public void unlockSystemUser() { + mBackupHandler.post(() -> { + Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup init"); + sInstance.initialize(UserHandle.USER_SYSTEM); + Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); + + // Migrate legacy setting + Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup migrate"); + if (!backupSettingMigrated(UserHandle.USER_SYSTEM)) { + if (DEBUG) { + Slog.i(TAG, "Backup enable apparently not migrated"); + } + final ContentResolver r = sInstance.mContext.getContentResolver(); + final int enableState = Settings.Secure.getIntForUser(r, + Settings.Secure.BACKUP_ENABLED, -1, UserHandle.USER_SYSTEM); + if (enableState >= 0) { + if (DEBUG) { + Slog.i(TAG, "Migrating enable state " + (enableState != 0)); + } + writeBackupEnableState(enableState != 0, UserHandle.USER_SYSTEM); + Settings.Secure.putStringForUser(r, + Settings.Secure.BACKUP_ENABLED, null, UserHandle.USER_SYSTEM); + } else { + if (DEBUG) { + Slog.i(TAG, "Backup not yet configured; retaining null enable state"); + } + } + } + Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); + + Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "backup enable"); + try { + sInstance.setBackupEnabled(readBackupEnableState(UserHandle.USER_SYSTEM)); + } catch (RemoteException e) { + // can't happen; it's a local object + } + Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); + }); + } + // Bookkeeping of in-flight operations for timeout etc. purposes. The operation // token is the index of the entry in the pending-operations list. public static final int OP_PENDING = 0; diff --git a/services/backup/java/com/android/server/backup/Trampoline.java b/services/backup/java/com/android/server/backup/Trampoline.java index 245bc1dfd3bf2..9739e38055b8b 100644 --- a/services/backup/java/com/android/server/backup/Trampoline.java +++ b/services/backup/java/com/android/server/backup/Trampoline.java @@ -139,6 +139,13 @@ public class Trampoline extends IBackupManager.Stub { } } + void unlockSystemUser() { + BackupManagerServiceInterface svc = mService; + if (svc != null) { + svc.unlockSystemUser(); + } + } + public void setBackupServiceActive(final int userHandle, boolean makeActive) { // Only the DPM should be changing the active state of backup final int caller = binderGetCallingUid();