Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a48795b3 authored by Ted Bauer's avatar Ted Bauer
Browse files

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
parent 4881667c

packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java

+20 −0
Original line number Diff line number Diff line
@@ -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);

packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java

+29 −27
Original line number Diff line number Diff line
@@ -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)) {

packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java

+14 −21
Original line number Diff line number Diff line
@@ -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() {