From afe55106e57111f57014f952a469a22b19a94603 Mon Sep 17 00:00:00 2001 From: Ted Bauer Date: Mon, 13 Nov 2023 19:06:36 +0000 Subject: [PATCH] Cache Java codegen'd flags in static member variables. By caching flag values directly in member variables instead of caching a HashMap and accessing that, flag reads avoid `hashCode()`, map lookup, and Boolean.parse runtime costs. Flag reads are turning out to have performance problems in hot paths, so this should help to alleviate that. Bug: 309625014 Test: m Change-Id: I923bf6af2ae3fcbbf2fee7126b492a47cd6049ad --- tools/aconfig/Android.bp | 1 + tools/aconfig/Cargo.toml | 1 + tools/aconfig/src/codegen_java.rs | 80 +++++++++---------- .../templates/FeatureFlagsImpl.java.template | 75 ++++++++--------- 4 files changed, 79 insertions(+), 78 deletions(-) diff --git a/tools/aconfig/Android.bp b/tools/aconfig/Android.bp index 1ad9053851..e2fadb05fb 100644 --- a/tools/aconfig/Android.bp +++ b/tools/aconfig/Android.bp @@ -58,6 +58,7 @@ rust_defaults { "libaconfig_protos", "libanyhow", "libclap", + "libitertools", "libprotobuf", "libserde", "libserde_json", diff --git a/tools/aconfig/Cargo.toml b/tools/aconfig/Cargo.toml index 941b30d909..2edf4b89cc 100644 --- a/tools/aconfig/Cargo.toml +++ b/tools/aconfig/Cargo.toml @@ -11,6 +11,7 @@ cargo = [] [dependencies] anyhow = "1.0.69" clap = { version = "4.1.8", features = ["derive"] } +itertools = "0.10.5" paste = "1.0.11" protobuf = "3.2.0" serde = { version = "1.0.152", features = ["derive"] } diff --git a/tools/aconfig/src/codegen_java.rs b/tools/aconfig/src/codegen_java.rs index 05ee0d759f..954eccc8f6 100644 --- a/tools/aconfig/src/codegen_java.rs +++ b/tools/aconfig/src/codegen_java.rs @@ -15,6 +15,7 @@ */ use anyhow::Result; +use itertools::Itertools; use serde::Serialize; use std::collections::BTreeSet; use std::path::PathBuf; @@ -34,12 +35,18 @@ where { let flag_elements: Vec = parsed_flags_iter.map(|pf| create_flag_element(package, pf)).collect(); + let namespace_set: BTreeSet = flag_elements + .iter() + .unique_by(|f| &f.device_config_namespace) + .map(|f| f.device_config_namespace.clone()) + .collect(); let properties_set: BTreeSet = flag_elements.iter().map(|fe| format_property_name(&fe.device_config_namespace)).collect(); let is_read_write = flag_elements.iter().any(|elem| elem.is_read_write); let is_test_mode = codegen_mode == CodegenMode::Test; let context = Context { flag_elements, + namespace_set, is_test_mode, is_read_write, properties_set, @@ -75,6 +82,7 @@ where #[derive(Serialize)] struct Context { pub flag_elements: Vec, + pub namespace_set: BTreeSet, pub is_test_mode: bool, pub is_read_write: bool, pub properties_set: BTreeSet, @@ -289,7 +297,31 @@ mod tests { import android.provider.DeviceConfig.Properties; /** @hide */ public final class FeatureFlagsImpl implements FeatureFlags { - private Properties mPropertiesAconfigTest; + private static boolean aconfig_test_is_cached = false; + private static boolean disabledRw = false; + private static boolean enabledRw = true; + + + private void load_overrides_aconfig_test() { + try { + Properties properties = DeviceConfig.getProperties("aconfig_test"); + disabledRw = + properties.getBoolean("com.android.aconfig.test.disabled_rw", false); + enabledRw = + properties.getBoolean("com.android.aconfig.test.enabled_rw", true); + } catch (NullPointerException e) { + throw new RuntimeException( + "Cannot read value from namespace aconfig_test " + + "from DeviceConfig. It could be that the code using flag " + + "executed before SettingsProvider initialization. Please use " + + "fixed read-only flag by adding is_fixed_read_only: true in " + + "flag declaration.", + e + ); + } + aconfig_test_is_cached = true; + } + @Override @UnsupportedAppUsage public boolean disabledRo() { @@ -298,18 +330,10 @@ mod tests { @Override @UnsupportedAppUsage public boolean disabledRw() { - if (mPropertiesAconfigTest == null) { - mPropertiesAconfigTest = - getProperties( - "aconfig_test", - "com.android.aconfig.test.disabled_rw" - ); + if (!aconfig_test_is_cached) { + load_overrides_aconfig_test(); } - return mPropertiesAconfigTest - .getBoolean( - "com.android.aconfig.test.disabled_rw", - false - ); + return disabledRw; } @Override @UnsupportedAppUsage @@ -324,36 +348,10 @@ mod tests { @Override @UnsupportedAppUsage public boolean enabledRw() { - if (mPropertiesAconfigTest == null) { - mPropertiesAconfigTest = - getProperties( - "aconfig_test", - "com.android.aconfig.test.enabled_rw" - ); + if (!aconfig_test_is_cached) { + load_overrides_aconfig_test(); } - return mPropertiesAconfigTest - .getBoolean( - "com.android.aconfig.test.enabled_rw", - true - ); - } - private Properties getProperties( - String namespace, - String flagName) { - Properties properties = null; - try { - properties = DeviceConfig.getProperties(namespace); - } catch (NullPointerException e) { - throw new RuntimeException( - "Cannot read value of flag " + flagName + " from DeviceConfig. " - + "It could be that the code using flag executed " - + "before SettingsProvider initialization. " - + "Please use fixed read-only flag by adding " - + "is_fixed_read_only: true in flag declaration.", - e - ); - } - return properties; + return enabledRw; } } "#; diff --git a/tools/aconfig/templates/FeatureFlagsImpl.java.template b/tools/aconfig/templates/FeatureFlagsImpl.java.template index ff089dfdc8..87d567c05b 100644 --- a/tools/aconfig/templates/FeatureFlagsImpl.java.template +++ b/tools/aconfig/templates/FeatureFlagsImpl.java.template @@ -8,10 +8,41 @@ import android.provider.DeviceConfig.Properties; {{ endif }} /** @hide */ public final class FeatureFlagsImpl implements FeatureFlags \{ -{{ if is_read_write- }} -{{ for properties in properties_set }} - private Properties {properties}; +{{- if is_read_write }} +{{- for namespace in namespace_set }} + private static boolean {namespace}_is_cached = false; +{{- endfor- }} + +{{ for flag in flag_elements }} +{{- if flag.is_read_write }} + private static boolean {flag.method_name} = {flag.default_value}; +{{- endif- }} {{ endfor }} + +{{ for namespace in namespace_set }} + private void load_overrides_{namespace}() \{ + try \{ + Properties properties = DeviceConfig.getProperties("{namespace}"); + + {{- for flag in flag_elements }} + {{- if flag.is_read_write }} + {flag.method_name} = + properties.getBoolean("{flag.device_config_flag}", {flag.default_value}); + {{- endif- }} + {{ endfor }} + } catch (NullPointerException e) \{ + throw new RuntimeException( + "Cannot read value from namespace {namespace} " + + "from DeviceConfig. It could be that the code using flag " + + "executed before SettingsProvider initialization. Please use " + + "fixed read-only flag by adding is_fixed_read_only: true in " + + "flag declaration.", + e + ); + } + {namespace}_is_cached = true; + } +{{ endfor- }} {{ endif- }} {{ for flag in flag_elements }} @@ -19,45 +50,15 @@ public final class FeatureFlagsImpl implements FeatureFlags \{ @UnsupportedAppUsage public boolean {flag.method_name}() \{ {{ -if flag.is_read_write }} - if ({flag.properties} == null) \{ - {flag.properties} = - getProperties( - "{flag.device_config_namespace}", - "{flag.device_config_flag}" - ); + if (!{flag.device_config_namespace}_is_cached) \{ + load_overrides_{flag.device_config_namespace}(); } - return {flag.properties} - .getBoolean( - "{flag.device_config_flag}", - {flag.default_value} - ); + return {flag.method_name}; {{ else }} return {flag.default_value}; {{ endif- }} } {{ endfor }} - -{{ -if is_read_write }} - private Properties getProperties( - String namespace, - String flagName) \{ - Properties properties = null; - try \{ - properties = DeviceConfig.getProperties(namespace); - } catch (NullPointerException e) \{ - throw new RuntimeException( - "Cannot read value of flag " + flagName + " from DeviceConfig. " - + "It could be that the code using flag executed " - + "before SettingsProvider initialization. " - + "Please use fixed read-only flag by adding " - + "is_fixed_read_only: true in flag declaration.", - e - ); - } - - return properties; - } -{{ endif- }} } {{ else }} {#- Generate only stub if in test mode #} @@ -70,6 +71,6 @@ public final class FeatureFlagsImpl implements FeatureFlags \{ throw new UnsupportedOperationException( "Method is not implemented."); } -{{ endfor }} +{{ endfor- }} } {{ endif }}