From a8f9026d22dae7fcb385bd06ed475a5dd36993c3 Mon Sep 17 00:00:00 2001 From: Svet Ganov Date: Tue, 10 May 2016 08:44:48 -0700 Subject: [PATCH] Persist settings on a dedicated background thread Settings were persisted on the system background thread but during first boot the device is under heavy load and persisting settings competes with other system components using the shared background thread. As a result persisting settings can be delayed much longer than the expected 200ms. This can cause issues with setup wizard being skipped/went over and its component disaabled being persisted but the setting whether the device is provisioned not being persisted - now if the device boots it will have no SUW but also the home button would be missing. Generally, we need a tansactional abstraction in the system process to peform all delayed operations atomically. bug:25472484 Change-Id: I8e0cf7ffa32e86e36e777964eb0c3cc7de02d3c3 --- .../providers/settings/SettingsProvider.java | 13 ++++++++++++- .../android/providers/settings/SettingsState.java | 6 ++++-- .../providers/settings/SettingsStateTest.java | 7 ++++--- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index 5ff9c0ce3c847..881e8defc12b3 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -46,6 +46,7 @@ import android.os.Debug; import android.os.DropBoxManager; import android.os.Environment; import android.os.Handler; +import android.os.HandlerThread; import android.os.Looper; import android.os.Message; import android.os.ParcelFileDescriptor; @@ -199,6 +200,12 @@ public class SettingsProvider extends ContentProvider { @GuardedBy("mLock") private SettingsRegistry mSettingsRegistry; + @GuardedBy("mLock") + private HandlerThread mHandlerThread; + + @GuardedBy("mLock") + private Handler mBackgroundHandler; + // We have to call in the user manager with no lock held, private volatile UserManager mUserManager; @@ -244,6 +251,10 @@ public class SettingsProvider extends ContentProvider { synchronized (mLock) { mUserManager = UserManager.get(getContext()); mPackageManager = AppGlobals.getPackageManager(); + mHandlerThread = new HandlerThread(LOG_TAG, + Process.THREAD_PRIORITY_BACKGROUND); + mHandlerThread.start(); + mBackgroundHandler = new Handler(mHandlerThread.getLooper()); mSettingsRegistry = new SettingsRegistry(); } registerBroadcastReceivers(); @@ -1669,7 +1680,7 @@ public class SettingsProvider extends ContentProvider { if (mSettingsStates.get(key) == null) { final int maxBytesPerPackage = getMaxBytesPerPackageForType(getTypeFromKey(key)); SettingsState settingsState = new SettingsState(mLock, getSettingsFile(key), key, - maxBytesPerPackage); + maxBytesPerPackage, mBackgroundHandler); mSettingsStates.put(key, settingsState); } } diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java index 2de0618c6564e..1e027475fd69d 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java @@ -95,7 +95,7 @@ final class SettingsState { private final Object mLock; - private final Handler mHandler = new MyHandler(); + private final Handler mHandler; @GuardedBy("mLock") private final ArrayMap mSettings = new ArrayMap<>(); @@ -134,13 +134,15 @@ final class SettingsState { @GuardedBy("mLock") private long mNextId; - public SettingsState(Object lock, File file, int key, int maxBytesPerAppPackage) { + public SettingsState(Object lock, File file, int key, int maxBytesPerAppPackage, + Handler handler) { // It is important that we use the same lock as the settings provider // to ensure multiple mutations on this state are atomicaly persisted // as the async persistence should be blocked while we make changes. mLock = lock; mStatePersistFile = file; mKey = key; + mHandler = handler; if (maxBytesPerAppPackage == MAX_BYTES_PER_APP_PACKAGE_LIMITED) { mMaxBytesPerAppPackage = maxBytesPerAppPackage; mPackageToMemoryUsage = new ArrayMap<>(); diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java index 3f9ffa10f9630..53c2958ad914a 100644 --- a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java +++ b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java @@ -15,6 +15,7 @@ */ package com.android.providers.settings; +import android.os.Handler; import android.test.AndroidTestCase; import android.util.Xml; @@ -126,7 +127,7 @@ public class SettingsStateTest extends AndroidTestCase { final Object lock = new Object(); final SettingsState ssWriter = new SettingsState(lock, file, 1, - SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED); + SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, new Handler()); ssWriter.setVersionLocked(SettingsState.SETTINGS_VERSOIN_NEW_ENCODING); ssWriter.insertSettingLocked("k1", "\u0000", "package"); @@ -138,7 +139,7 @@ public class SettingsStateTest extends AndroidTestCase { } final SettingsState ssReader = new SettingsState(lock, file, 1, - SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED); + SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, new Handler()); synchronized (lock) { assertEquals("\u0000", ssReader.getSettingLocked("k1").getValue()); assertEquals("abc", ssReader.getSettingLocked("k2").getValue()); @@ -165,7 +166,7 @@ public class SettingsStateTest extends AndroidTestCase { os.close(); final SettingsState ss = new SettingsState(lock, file, 1, - SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED); + SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, new Handler()); synchronized (lock) { SettingsState.Setting s; s = ss.getSettingLocked("k0");