From d05c0ff623ce7362ad718e51b378805450928fe9 Mon Sep 17 00:00:00 2001 From: Songchun Fan Date: Tue, 9 Jan 2024 16:03:16 -0800 Subject: [SettingsProvider] use per-prefix caches for Config.getStrings queries Previously all values are cached in the same map, which is inefficient for Config.getStrings queries which request for multiple settings under the same prefix and we have look-up the whole cache multiple times. It is better to have per-prefix caches which only requires one look up per prefix, and subsequent searches inside the prefix cache would be faster as well because each per-prefix cache is much smaller. BUG: 316866951 Test: atest CtsDeviceConfigTestCases CtsPermissionUiTestCases CtsDisplayTestCases CtsAutoFillServiceTestCases SettingsProviderTest CtsProviderTestCases Change-Id: Ifb4ff91cc034405227d9614bf1029d2a70901bce --- core/java/android/provider/Settings.java | 93 +++++++++++++++----------------- 1 file changed, 42 insertions(+), 51 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index f974ef43a9dc..5030209d1942 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -3238,9 +3238,17 @@ public final class Settings { private static final String NAME_EQ_PLACEHOLDER = "name=?"; + // Cached values of queried settings. + // Key is the setting's name, value is the setting's value. // Must synchronize on 'this' to access mValues and mValuesVersion. private final ArrayMap mValues = new ArrayMap<>(); + // Cached values for queried prefixes. + // Key is the prefix, value is all of the settings under the prefix, mapped from a setting's + // name to a setting's value. The name string doesn't include the prefix. + // Must synchronize on 'this' to access. + private final ArrayMap> mPrefixToValues = new ArrayMap<>(); + private final Uri mUri; @UnsupportedAppUsage private final ContentProviderHolder mProviderHolder; @@ -3585,15 +3593,13 @@ public final class Settings { || applicationInfo.isSignedWithPlatformKey(); } - private ArrayMap getStringsForPrefixStripPrefix( - ContentResolver cr, String prefix, String[] names) { + private Map getStringsForPrefixStripPrefix( + ContentResolver cr, String prefix, List names) { String namespace = prefix.substring(0, prefix.length() - 1); ArrayMap keyValues = new ArrayMap<>(); int substringLength = prefix.length(); - int currentGeneration = -1; boolean needsGenerationTracker = false; - synchronized (NameValueCache.this) { final GenerationTracker generationTracker = mGenerationTrackers.get(prefix); if (generationTracker != null) { @@ -3607,40 +3613,22 @@ public final class Settings { // generation tracker and request a new one generationTracker.destroy(); mGenerationTrackers.remove(prefix); - for (int i = mValues.size() - 1; i >= 0; i--) { - String key = mValues.keyAt(i); - if (key.startsWith(prefix)) { - mValues.remove(key); - } - } + mPrefixToValues.remove(prefix); needsGenerationTracker = true; } else { - boolean prefixCached = mValues.containsKey(prefix); - if (prefixCached) { - if (DEBUG) { - Log.i(TAG, "Cache hit for prefix:" + prefix); - } - if (names.length > 0) { + final ArrayMap cachedSettings = mPrefixToValues.get(prefix); + if (cachedSettings != null) { + if (!names.isEmpty()) { for (String name : names) { - // mValues can contain "null" values, need to use containsKey. - if (mValues.containsKey(name)) { + // The cache can contain "null" values, need to use containsKey. + if (cachedSettings.containsKey(name)) { keyValues.put( - name.substring(substringLength), - mValues.get(name)); + name, + cachedSettings.get(name)); } } } else { - for (int i = 0; i < mValues.size(); ++i) { - String key = mValues.keyAt(i); - // Explicitly exclude the prefix as it is only there to - // signal that the prefix has been cached. - if (key.startsWith(prefix) && !key.equals(prefix)) { - String value = mValues.valueAt(i); - keyValues.put( - key.substring(substringLength), - value); - } - } + keyValues.putAll(cachedSettings); } return keyValues; } @@ -3650,7 +3638,6 @@ public final class Settings { needsGenerationTracker = true; } } - if (mCallListCommand == null) { // No list command specified, return empty map return keyValues; @@ -3695,20 +3682,23 @@ public final class Settings { } // All flags for the namespace - Map flagsToValues = + HashMap flagsToValues = (HashMap) b.getSerializable(Settings.NameValueTable.VALUE, java.util.HashMap.class); + if (flagsToValues == null) { + return keyValues; + } // Only the flags requested by the caller - if (names.length > 0) { + if (!names.isEmpty()) { for (String name : names) { // flagsToValues can contain "null" values, need to use containsKey. - if (flagsToValues.containsKey(name)) { + final String key = Config.createCompositeName(namespace, name); + if (flagsToValues.containsKey(key)) { keyValues.put( - name.substring(substringLength), - flagsToValues.get(name)); + name, + flagsToValues.get(key)); } } } else { - keyValues.ensureCapacity(keyValues.size() + flagsToValues.size()); for (Map.Entry flag : flagsToValues.entrySet()) { keyValues.put( flag.getKey().substring(substringLength), @@ -3742,10 +3732,18 @@ public final class Settings { if (DEBUG) { Log.i(TAG, "Updating cache for prefix:" + prefix); } - // cache the complete list of flags for the namespace - mValues.putAll(flagsToValues); - // Adding the prefix as a signal that the prefix is cached. - mValues.put(prefix, null); + // Cache the complete list of flags for the namespace for bulk queries. + // In this cached list, the setting's name doesn't include the prefix. + ArrayMap namesToValues = + new ArrayMap<>(flagsToValues.size() + 1); + for (Map.Entry flag : flagsToValues.entrySet()) { + namesToValues.put( + flag.getKey().substring(substringLength), + flag.getValue()); + } + // Legacy behavior, we return <"", null> when queried with name = "" + namesToValues.put("", null); + mPrefixToValues.put(prefix, namesToValues); } } return keyValues; @@ -19909,16 +19907,9 @@ public final class Settings { @RequiresPermission(Manifest.permission.READ_DEVICE_CONFIG) public static Map getStrings(@NonNull ContentResolver resolver, @NonNull String namespace, @NonNull List names) { - String[] compositeNames = new String[names.size()]; - for (int i = 0, size = names.size(); i < size; ++i) { - compositeNames[i] = createCompositeName(namespace, names.get(i)); - } - String prefix = createPrefix(namespace); - ArrayMap keyValues = sNameValueCache.getStringsForPrefixStripPrefix( - resolver, prefix, compositeNames); - return keyValues; + return sNameValueCache.getStringsForPrefixStripPrefix(resolver, prefix, names); } /** @@ -20240,7 +20231,7 @@ public final class Settings { } } - private static String createCompositeName(@NonNull String namespace, @NonNull String name) { + static String createCompositeName(@NonNull String namespace, @NonNull String name) { Preconditions.checkNotNull(namespace); Preconditions.checkNotNull(name); var sb = new StringBuilder(namespace.length() + 1 + name.length()); -- cgit v1.2.3-59-g8ed1b