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

Commit da7a024d authored by Dennis Shen's avatar Dennis Shen
Browse files

remove unecessary staging to sys prop. 

If the staged value is the same as the current server side value, thre
is no need to stage. Without this optimization, we can run into sys prop
performance issue at boot.

Bug: b/332611029
Test: m
Change-Id: I253e8ee0a8619cd66c1ad104db9e6c3b5a25da42
parent 39fce101

services/core/java/com/android/server/am/SettingsToPropertiesMapper.java

+81 −34
Original line number Diff line number Diff line
@@ -35,6 +35,10 @@ import java.io.File; 
import java.io.FileReader;

import java.io.IOException;

import java.util.HashSet;

import java.util.HashMap;

import java.util.Map;

import java.util.List;

import java.util.ArrayList;



/**

 * Maps system settings to system properties.
@@ -320,15 +324,30 @@ public class SettingsToPropertiesMapper { 
            NAMESPACE_REBOOT_STAGING,

            AsyncTask.THREAD_POOL_EXECUTOR,

            (DeviceConfig.Properties properties) -> {

              String scope = properties.getNamespace();

              for (String key : properties.getKeyset()) {

                String aconfigPropertyName = makeAconfigFlagStagedPropertyName(key);

                if (aconfigPropertyName == null) {

                    log("unable to construct system property for " + scope + "/" + key);

                    return;



              HashMap<String, HashMap<String, String>> propsToStage =

                  getStagedFlagsWithValueChange(properties);



              for (HashMap.Entry<String, HashMap<String, String>> entry : propsToStage.entrySet()) {

                String actualNamespace = entry.getKey();

                HashMap<String, String> flagValuesToStage = entry.getValue();



                for (String flagName : flagValuesToStage.keySet()) {

                  String stagedValue = flagValuesToStage.get(flagName);

                  String propertyName = "next_boot." + makeAconfigFlagPropertyName(

                      actualNamespace, flagName);



                  if (!propertyName.matches(SYSTEM_PROPERTY_VALID_CHARACTERS_REGEX)

                      || propertyName.contains(SYSTEM_PROPERTY_INVALID_SUBSTRING)) {

                    log("unable to construct system property for " + actualNamespace

                        + "/" + flagName);

                    continue;

                  }



                  setProperty(propertyName, stagedValue);

                }

                setProperty(aconfigPropertyName, properties.getString(key, null));

              }



            });

    }
@@ -401,25 +420,18 @@ public class SettingsToPropertiesMapper { 
    }



    /**

     * system property name constructing rule for staged aconfig flags, the flag name

     * is in the form of [namespace]*[actual flag name], we should push the following

     * to system properties

     * "next_boot.[actual sys prop name]".

     * system property name constructing rule for aconfig flags:

     * "persist.device_config.aconfig_flags.[category_name].[flag_name]".

     * If the name contains invalid characters or substrings for system property name,

     * will return null.

     * @param categoryName

     * @param flagName

     * @return

     */

    @VisibleForTesting

    static String makeAconfigFlagStagedPropertyName(String flagName) {

        int idx = flagName.indexOf(NAMESPACE_REBOOT_STAGING_DELIMITER);

        if (idx == -1 || idx == flagName.length() - 1 || idx == 0) {

            log("invalid staged flag: " + flagName);

            return null;

        }



        String propertyName = "next_boot." + makeAconfigFlagPropertyName(

                flagName.substring(0, idx), flagName.substring(idx+1));

    static String makeAconfigFlagPropertyName(String categoryName, String flagName) {

        String propertyName = SYSTEM_PROPERTY_PREFIX + "aconfig_flags." +

                              categoryName + "." + flagName;



        if (!propertyName.matches(SYSTEM_PROPERTY_VALID_CHARACTERS_REGEX)

                || propertyName.contains(SYSTEM_PROPERTY_INVALID_SUBSTRING)) {
@@ -430,25 +442,60 @@ public class SettingsToPropertiesMapper { 
    }



    /**

     * system property name constructing rule for aconfig flags:

     * "persist.device_config.aconfig_flags.[category_name].[flag_name]".

     * If the name contains invalid characters or substrings for system property name,

     * will return null.

     * @param categoryName

     * @param flagName

     * @return

     * Get the flags that need to be staged in sys prop, only these with a real value

     * change needs to be staged in sys prop. Otherwise, the flag stage is useless and

     * create performance problem at sys prop side.

     * @param properties

     * @return a hash map of namespace name to actual flags to stage

     */

    @VisibleForTesting

    static String makeAconfigFlagPropertyName(String categoryName, String flagName) {

        String propertyName = SYSTEM_PROPERTY_PREFIX + "aconfig_flags." +

                              categoryName + "." + flagName;

    static HashMap<String, HashMap<String, String>> getStagedFlagsWithValueChange(

        DeviceConfig.Properties properties) {



        if (!propertyName.matches(SYSTEM_PROPERTY_VALID_CHARACTERS_REGEX)

                || propertyName.contains(SYSTEM_PROPERTY_INVALID_SUBSTRING)) {

            return null;

      // sort flags by actual namespace of the flag

      HashMap<String, HashMap<String, String>> stagedProps = new HashMap<>();

      for (String flagName : properties.getKeyset()) {

        int idx = flagName.indexOf(NAMESPACE_REBOOT_STAGING_DELIMITER);

        if (idx == -1 || idx == flagName.length() - 1 || idx == 0) {

          log("invalid staged flag: " + flagName);

          continue;

        }

        String actualNamespace = flagName.substring(0, idx);

        String actualFlagName = flagName.substring(idx+1);

        HashMap<String, String> flagStagedValues = stagedProps.get(actualNamespace);

        if (flagStagedValues == null) {

          flagStagedValues = new HashMap<String, String>();

          stagedProps.put(actualNamespace, flagStagedValues);

        }

        flagStagedValues.put(actualFlagName, properties.getString(flagName, null));

      }



        return propertyName;

      // for each namespace, find flags with real flag value change

      HashMap<String, HashMap<String, String>> propsToStage = new HashMap<>();

      for (HashMap.Entry<String, HashMap<String, String>> entry : stagedProps.entrySet()) {

        String actualNamespace = entry.getKey();

        HashMap<String, String> flagStagedValues = entry.getValue();

        Map<String, String> flagCurrentValues = Settings.Config.getStrings(

            actualNamespace, new ArrayList<String>(flagStagedValues.keySet()));



        HashMap<String, String> flagsToStage = new HashMap<>();

        for (String flagName : flagStagedValues.keySet()) {

          String stagedValue = flagStagedValues.get(flagName);

          String currentValue = flagCurrentValues.get(flagName);

          if (currentValue == null) {

            currentValue = new String("false");

          }

          if (stagedValue != null && !stagedValue.equalsIgnoreCase(currentValue)) {

            flagsToStage.put(flagName, stagedValue);

          }

        }



        if (!flagsToStage.isEmpty()) {

          propsToStage.put(actualNamespace, flagsToStage);

        }

      }



      return propsToStage;

    }



    private void setProperty(String key, String value) {

services/tests/mockingservicestests/src/com/android/server/am/SettingsToPropertiesMapperTest.java

+60 −1
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; 
import android.content.ContentResolver;

import android.os.SystemProperties;

import android.provider.Settings;

import android.provider.DeviceConfig.Properties;

import android.text.TextUtils;



import com.android.dx.mockito.inline.extended.ExtendedMockito;
@@ -42,6 +43,7 @@ import java.util.Arrays; 
import java.util.HashMap;

import java.util.HashSet;

import java.util.List;

import java.util.Map;



/**

 * Test SettingsToPropertiesMapper.
@@ -61,6 +63,7 @@ public class SettingsToPropertiesMapperTest { 


    private HashMap<String, String> mSystemSettingsMap;

    private HashMap<String, String> mGlobalSettingsMap;

    private HashMap<String, String> mConfigSettingsMap;



    @Before

    public void setUp() throws Exception {
@@ -71,9 +74,11 @@ public class SettingsToPropertiesMapperTest { 
                        .spyStatic(SystemProperties.class)

                        .spyStatic(Settings.Global.class)

                        .spyStatic(SettingsToPropertiesMapper.class)

                        .spyStatic(Settings.Config.class)

                        .startMocking();

        mSystemSettingsMap = new HashMap<>();

        mGlobalSettingsMap = new HashMap<>();

        mConfigSettingsMap = new HashMap<>();



        // Mock SystemProperties setter and various getters

        doAnswer((Answer<Void>) invocationOnMock -> {
@@ -93,7 +98,7 @@ public class SettingsToPropertiesMapperTest { 
                }

        ).when(() -> SystemProperties.get(anyString()));



        // Mock Settings.Global methods

        // Mock Settings.Global method

        doAnswer((Answer<String>) invocationOnMock -> {

                    String key = invocationOnMock.getArgument(1);
@@ -101,6 +106,21 @@ public class SettingsToPropertiesMapperTest { 
                }

        ).when(() -> Settings.Global.getString(any(), anyString()));



        // Mock Settings.Config getstrings method

        doAnswer((Answer<Map<String, String>>) invocationOnMock -> {

                    String namespace = invocationOnMock.getArgument(0);

                    List<String> flags = invocationOnMock.getArgument(1);

                    HashMap<String, String> values = new HashMap<>();

                    for (String flag : flags) {

                      String value = mConfigSettingsMap.get(namespace + "/" + flag);

                      if (value != null) {

                        values.put(flag, value);

                      }

                    }

                    return values;

                }

        ).when(() -> Settings.Config.getStrings(anyString(), any()));



        mTestMapper = new SettingsToPropertiesMapper(

            mMockContentResolver, TEST_MAPPING, new String[] {}, new String[] {});

    }
@@ -239,4 +259,43 @@ public class SettingsToPropertiesMapperTest { 
        Assert.assertTrue(categories.contains("category2"));

        Assert.assertTrue(categories.contains("category3"));

    }



  @Test

  public void testGetStagedFlagsWithValueChange() {

    // mock up what is in the setting already

    mConfigSettingsMap.put("namespace_1/flag_1", "true");

    mConfigSettingsMap.put("namespace_1/flag_2", "true");



    // mock up input

    String namespace = "staged";

    Map<String, String> keyValueMap = new HashMap<>();

    // case 1: existing prop, stage the same value

    keyValueMap.put("namespace_1*flag_1", "true");

    // case 2: existing prop, stage a different value

    keyValueMap.put("namespace_1*flag_2", "false");

    // case 3: new prop, stage the non default value

    keyValueMap.put("namespace_2*flag_1", "true");

    // case 4: new prop, stage the default value

    keyValueMap.put("namespace_2*flag_2", "false");

    Properties props = new Properties(namespace, keyValueMap);



    HashMap<String, HashMap<String, String>> toStageProps =

        SettingsToPropertiesMapper.getStagedFlagsWithValueChange(props);



    HashMap<String, String> namespace_1_to_stage = toStageProps.get("namespace_1");

    HashMap<String, String> namespace_2_to_stage = toStageProps.get("namespace_2");

    Assert.assertTrue(namespace_1_to_stage != null);

    Assert.assertTrue(namespace_2_to_stage != null);



    String namespace_1_flag_1 = namespace_1_to_stage.get("flag_1");

    String namespace_1_flag_2 = namespace_1_to_stage.get("flag_2");

    String namespace_2_flag_1 = namespace_2_to_stage.get("flag_1");

    String namespace_2_flag_2 = namespace_2_to_stage.get("flag_2");

    Assert.assertTrue(namespace_1_flag_1 == null);

    Assert.assertTrue(namespace_1_flag_2 != null);

    Assert.assertTrue(namespace_2_flag_1 != null);

    Assert.assertTrue(namespace_2_flag_2 == null);

    Assert.assertTrue(namespace_1_flag_2.equals("false"));

    Assert.assertTrue(namespace_2_flag_1.equals("true"));

  }

}