Optimize default value loading.

Previously, we were loading all aconfig default values
into the ArrayMap backing SettingsProvider. This was
causing boot regressions. It was likely because this
would make any calls to `getAllConfigFlags` much more
expensive; this function was looping through all
SettingsProvider flags and doing a prefix check, _every
time_ it was called.

This change loads defaults into buckets keyed by the
namespaces of the defaults, in a new in-memory structure.
This way, `getAllConfigFlags` needs to just look up the
defaults for its namespace, when doing a namespace-level
call. This doesn't optimize anything when we get every
single flag, but from some investigation, there's never
a time this gets invoked in the boot process.

Test: atest SettingsStateTest
Bug: 318514727
Change-Id: I3915ae589de01a09cd959c8b79859f7e4ce65285

3 files changed, 63 insertions, 48 deletions

diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
index b0abf92ffe08..2d442f4c0e6e 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
@@ -1349,6 +1349,26 @@ public class SettingsProvider extends ContentProvider {
             final int nameCount = names.size();
             HashMap<String, String> flagsToValues = new HashMap<>(names.size());
 
+            if (Flags.loadAconfigDefaults()) {
+                Map<String, Map<String, String>> allDefaults =
+                        settingsState.getAconfigDefaultValues();
+
+                if (allDefaults != null) {
+                    if (prefix != null) {
+                        String namespace = prefix.substring(0, prefix.length() - 1);
+
+                        Map<String, String> namespaceDefaults = allDefaults.get(namespace);
+                        if (namespaceDefaults != null) {
+                            flagsToValues.putAll(namespaceDefaults);
+                        }
+                    } else {
+                        for (Map<String, String> namespaceDefaults : allDefaults.values()) {
+                            flagsToValues.putAll(namespaceDefaults);
+                        }
+                    }
+                }
+            }
+
             for (int i = 0; i < nameCount; i++) {
                 String name = names.get(i);
                 Setting setting = settingsState.getSettingLocked(name);
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
index 73c2e22240d3..6f3c88fc8706 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
@@ -69,6 +69,7 @@ import java.io.PrintWriter;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -236,6 +237,10 @@ final class SettingsState {
     @GuardedBy("mLock")
     private int mNextHistoricalOpIdx;
 
+    @GuardedBy("mLock")
+    @Nullable
+    private Map<String, Map<String, String>> mNamespaceDefaults;
+
     public static final int SETTINGS_TYPE_GLOBAL = 0;
     public static final int SETTINGS_TYPE_SYSTEM = 1;
     public static final int SETTINGS_TYPE_SECURE = 2;
@@ -331,25 +336,21 @@ final class SettingsState {
             readStateSyncLocked();
 
             if (Flags.loadAconfigDefaults()) {
-                // Only load aconfig defaults if this is the first boot, the XML
-                // file doesn't exist yet, or this device is on its first boot after
-                // an OTA.
-                boolean shouldLoadAconfigValues = isConfigSettingsKey(mKey)
-                        && (!file.exists()
-                                || mContext.getPackageManager().isDeviceUpgrading());
-                if (shouldLoadAconfigValues) {
+                if (isConfigSettingsKey(mKey)) {
                     loadAconfigDefaultValuesLocked();
                 }
             }
+
         }
     }
 
     @GuardedBy("mLock")
     private void loadAconfigDefaultValuesLocked() {
+        mNamespaceDefaults = new HashMap<>();
+
         for (String fileName : sAconfigTextProtoFilesOnDevice) {
             try (FileInputStream inputStream = new FileInputStream(fileName)) {
-                byte[] contents = inputStream.readAllBytes();
-                loadAconfigDefaultValues(contents);
+                loadAconfigDefaultValues(inputStream.readAllBytes(), mNamespaceDefaults);
             } catch (IOException e) {
                 Slog.e(LOG_TAG, "failed to read protobuf", e);
             }
@@ -358,27 +359,21 @@ final class SettingsState {
 
     @VisibleForTesting
     @GuardedBy("mLock")
-    public void loadAconfigDefaultValues(byte[] fileContents) {
+    public static void loadAconfigDefaultValues(byte[] fileContents,
+            @NonNull Map<String, Map<String, String>> defaultMap) {
         try {
-            parsed_flags parsedFlags = parsed_flags.parseFrom(fileContents);
-
-            if (parsedFlags == null) {
-                Slog.e(LOG_TAG, "failed to parse aconfig protobuf");
-                return;
-            }
-
+            parsed_flags parsedFlags =
+                    parsed_flags.parseFrom(fileContents);
             for (parsed_flag flag : parsedFlags.getParsedFlagList()) {
-                String flagName = flag.getNamespace() + "/"
-                        + flag.getPackage() + "." + flag.getName();
-                String value = flag.getState() == flag_state.ENABLED ? "true" : "false";
-
-                Setting existingSetting = getSettingLocked(flagName);
-                boolean isDefaultLoaded = existingSetting.getTag() != null
-                        && existingSetting.getTag().equals(BOOT_LOADED_DEFAULT_TAG);
-                if (existingSetting.getValue() == null || isDefaultLoaded) {
-                    insertSettingLocked(flagName, value, BOOT_LOADED_DEFAULT_TAG,
-                            false, flag.getPackage());
+                if (!defaultMap.containsKey(flag.getNamespace())) {
+                    Map<String, String> defaults = new HashMap<>();
+                    defaultMap.put(flag.getNamespace(), defaults);
                 }
+                String flagName = flag.getNamespace()
+                        + "/" + flag.getPackage() + "." + flag.getName();
+                String flagValue = flag.getState() == flag_state.ENABLED
+                        ? "true" : "false";
+                defaultMap.get(flag.getNamespace()).put(flagName, flagValue);
             }
         } catch (IOException e) {
             Slog.e(LOG_TAG, "failed to parse protobuf", e);
@@ -443,6 +438,13 @@ final class SettingsState {
         return names;
     }
 
+    @Nullable
+    public Map<String, Map<String, String>> getAconfigDefaultValues() {
+        synchronized (mLock) {
+            return mNamespaceDefaults;
+        }
+    }
+
     // The settings provider must hold its lock when calling here.
     public Setting getSettingLocked(String name) {
         if (TextUtils.isEmpty(name)) {
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 24625eaa5e13..e55bbecb67d7 100644
--- a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java
+++ b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java
@@ -30,6 +30,8 @@ import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.PrintStream;
+import java.util.HashMap;
+import java.util.Map;
 
 public class SettingsStateTest extends AndroidTestCase {
     public static final String CRAZY_STRING =
@@ -93,7 +95,6 @@ public class SettingsStateTest extends AndroidTestCase {
         SettingsState settingsState = new SettingsState(
                 getContext(), lock, mSettingsFile, configKey,
                 SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper());
-
         parsed_flags flags = parsed_flags
                 .newBuilder()
                 .addParsedFlag(parsed_flag
@@ -117,18 +118,13 @@ public class SettingsStateTest extends AndroidTestCase {
                 .build();
 
         synchronized (lock) {
-            settingsState.loadAconfigDefaultValues(flags.toByteArray());
-            settingsState.persistSettingsLocked();
-        }
-        settingsState.waitForHandler();
+            Map<String, Map<String, String>> defaults = new HashMap<>();
+            settingsState.loadAconfigDefaultValues(flags.toByteArray(), defaults);
+            Map<String, String> namespaceDefaults = defaults.get("test_namespace");
+            assertEquals(2, namespaceDefaults.keySet().size());
 
-        synchronized (lock) {
-            assertEquals("false",
-                    settingsState.getSettingLocked(
-                        "test_namespace/com.android.flags.flag1").getValue());
-            assertEquals("true",
-                    settingsState.getSettingLocked(
-                        "test_namespace/com.android.flags.flag2").getValue());
+            assertEquals("false", namespaceDefaults.get("test_namespace/com.android.flags.flag1"));
+            assertEquals("true", namespaceDefaults.get("test_namespace/com.android.flags.flag2"));
         }
     }
 
@@ -150,21 +146,18 @@ public class SettingsStateTest extends AndroidTestCase {
                 .build();
 
         synchronized (lock) {
-            settingsState.loadAconfigDefaultValues(flags.toByteArray());
-            settingsState.persistSettingsLocked();
-        }
-        settingsState.waitForHandler();
+            Map<String, Map<String, String>> defaults = new HashMap<>();
+            settingsState.loadAconfigDefaultValues(flags.toByteArray(), defaults);
 
-        synchronized (lock) {
-            assertEquals(null,
-                    settingsState.getSettingLocked(
-                        "test_namespace/com.android.flags.flag1").getValue());
+            Map<String, String> namespaceDefaults = defaults.get("test_namespace");
+            assertEquals(null, namespaceDefaults);
         }
     }
 
     public void testInvalidAconfigProtoDoesNotCrash() {
+        Map<String, Map<String, String>> defaults = new HashMap<>();
         SettingsState settingsState = getSettingStateObject();
-        settingsState.loadAconfigDefaultValues("invalid protobuf".getBytes());
+        settingsState.loadAconfigDefaultValues("invalid protobuf".getBytes(), defaults);
     }
 
     public void testIsBinary() {