From 96506f4349b88c01e10ef3691fece4215d70b3d5 Mon Sep 17 00:00:00 2001 From: Dennis Shen Date: Wed, 8 Nov 2023 19:17:49 +0000 Subject: [PATCH 1/2] aconfig: Cache flag values for c/c++ codegen Bug: b/307336730 Test: atest aconfig.test Change-Id: Id604cf154d09a48f657277af6d799f0e17bc4e93 --- tools/aconfig/src/codegen_cpp.rs | 43 ++++++++++++++----- .../templates/cpp_exported_header.template | 8 +++- .../templates/cpp_source_file.template | 14 +++--- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/tools/aconfig/src/codegen_cpp.rs b/tools/aconfig/src/codegen_cpp.rs index aeb57a3126..9e77b45d1c 100644 --- a/tools/aconfig/src/codegen_cpp.rs +++ b/tools/aconfig/src/codegen_cpp.rs @@ -31,9 +31,10 @@ pub fn generate_cpp_code<'a, I>( where I: Iterator, { + let mut readwrite_count = 0; let class_elements: Vec = - parsed_flags_iter.map(|pf| create_class_element(package, pf)).collect(); - let readwrite = class_elements.iter().any(|item| item.readwrite); + parsed_flags_iter.map(|pf| create_class_element(package, pf, &mut readwrite_count)).collect(); + let readwrite = readwrite_count > 0; let has_fixed_read_only = class_elements.iter().any(|item| item.is_fixed_read_only); let header = package.replace('.', "_"); let package_macro = header.to_uppercase(); @@ -46,6 +47,7 @@ where package, has_fixed_read_only, readwrite, + readwrite_count, for_test: codegen_mode == CodegenMode::Test, class_elements, }; @@ -88,12 +90,14 @@ pub struct Context<'a> { pub package: &'a str, pub has_fixed_read_only: bool, pub readwrite: bool, + pub readwrite_count: i32, pub for_test: bool, pub class_elements: Vec, } #[derive(Serialize)] pub struct ClassElement { + pub readwrite_idx: i32, pub readwrite: bool, pub is_fixed_read_only: bool, pub default_value: String, @@ -103,8 +107,13 @@ pub struct ClassElement { pub device_config_flag: String, } -fn create_class_element(package: &str, pf: &ProtoParsedFlag) -> ClassElement { +fn create_class_element(package: &str, pf: &ProtoParsedFlag, rw_count: &mut i32) -> ClassElement { ClassElement { + readwrite_idx: if pf.permission() == ProtoFlagPermission::READ_WRITE { + let index = *rw_count; *rw_count += 1; index + } else { + -1 + }, readwrite: pf.permission() == ProtoFlagPermission::READ_WRITE, is_fixed_read_only: pf.is_fixed_read_only(), default_value: if pf.state() == ProtoFlagState::ENABLED { @@ -139,8 +148,12 @@ mod tests { #ifdef __cplusplus #include +#include namespace com::android::aconfig::test { + +extern std::vector cache_; + class flag_provider_interface { public: virtual ~flag_provider_interface() = default; @@ -330,10 +343,13 @@ namespace com::android::aconfig::test { } virtual bool disabled_rw() override { - return server_configurable_flags::GetServerConfigurableFlag( - "aconfig_flags.aconfig_test", - "com.android.aconfig.test.disabled_rw", - "false") == "true"; + if (cache_[0] == -1) { + cache_[0] = server_configurable_flags::GetServerConfigurableFlag( + "aconfig_flags.aconfig_test", + "com.android.aconfig.test.disabled_rw", + "false") == "true"; + } + return cache_[0]; } virtual bool enabled_fixed_ro() override { @@ -345,14 +361,19 @@ namespace com::android::aconfig::test { } virtual bool enabled_rw() override { - return server_configurable_flags::GetServerConfigurableFlag( - "aconfig_flags.aconfig_test", - "com.android.aconfig.test.enabled_rw", - "true") == "true"; + if (cache_[1] == -1) { + cache_[1] = server_configurable_flags::GetServerConfigurableFlag( + "aconfig_flags.aconfig_test", + "com.android.aconfig.test.enabled_rw", + "true") == "true"; + } + return cache_[1]; } }; + std::vector cache_ = std::vector(2, -1); + std::unique_ptr provider_ = std::make_unique(); } diff --git a/tools/aconfig/templates/cpp_exported_header.template b/tools/aconfig/templates/cpp_exported_header.template index 6413699ffd..d19c0faccf 100644 --- a/tools/aconfig/templates/cpp_exported_header.template +++ b/tools/aconfig/templates/cpp_exported_header.template @@ -18,10 +18,16 @@ #ifdef __cplusplus #include - +{{ if not for_test- }} +#include +{{ -endif }} namespace {cpp_namespace} \{ +{{ if not for_test- }} +extern std::vector cache_; +{{ -endif }} + class flag_provider_interface \{ public: virtual ~flag_provider_interface() = default; diff --git a/tools/aconfig/templates/cpp_source_file.template b/tools/aconfig/templates/cpp_source_file.template index 0f1b845786..91e828aa3c 100644 --- a/tools/aconfig/templates/cpp_source_file.template +++ b/tools/aconfig/templates/cpp_source_file.template @@ -53,10 +53,13 @@ namespace {cpp_namespace} \{ {{ for item in class_elements}} virtual bool {item.flag_name}() override \{ {{ if item.readwrite- }} - return server_configurable_flags::GetServerConfigurableFlag( - "aconfig_flags.{item.device_config_namespace}", - "{item.device_config_flag}", - "{item.default_value}") == "true"; + if (cache_[{item.readwrite_idx}] == -1) \{ + cache_[{item.readwrite_idx}] = server_configurable_flags::GetServerConfigurableFlag( + "aconfig_flags.{item.device_config_namespace}", + "{item.device_config_flag}", + "{item.default_value}") == "true"; + } + return cache_[{item.readwrite_idx}]; {{ -else- }} {{ if item.is_fixed_read_only }} return {package_macro}_{item.flag_macro}; @@ -68,13 +71,14 @@ namespace {cpp_namespace} \{ {{ endfor }} }; + std::vector cache_ = std::vector({readwrite_count}, -1); {{ -endif }} - std::unique_ptr provider_ = std::make_unique(); + } From 737b8e30a190395d5011e2fd7a0e4a070dd0f08b Mon Sep 17 00:00:00 2001 From: Dennis Shen Date: Wed, 8 Nov 2023 19:58:47 +0000 Subject: [PATCH 2/2] aconfig: Cache flag values for rust codegen Bug: b/307336730 Test: atest aconfig.test Change-Id: I01741a4205cbe4e9b007f43b043505bcbcf05cd8 --- tools/aconfig/src/codegen_rust.rs | 27 +++++++++++++++------- tools/aconfig/templates/rust_prod.template | 19 +++++++++++---- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/tools/aconfig/src/codegen_rust.rs b/tools/aconfig/src/codegen_rust.rs index 4e4c7dd58a..78e62ba095 100644 --- a/tools/aconfig/src/codegen_rust.rs +++ b/tools/aconfig/src/codegen_rust.rs @@ -32,10 +32,12 @@ where { let template_flags: Vec = parsed_flags_iter.map(|pf| TemplateParsedFlag::new(package, pf)).collect(); + let has_readwrite = template_flags.iter().any(|item| item.readwrite); let context = TemplateContext { package: package.to_string(), template_flags, modules: package.split('.').map(|s| s.to_string()).collect::>(), + has_readwrite, }; let mut template = TinyTemplate::new(); template.add_template( @@ -55,6 +57,7 @@ struct TemplateContext { pub package: String, pub template_flags: Vec, pub modules: Vec, + pub has_readwrite: bool, } #[derive(Serialize)] @@ -94,6 +97,20 @@ mod tests { /// flag provider pub struct FlagProvider; +lazy_static::lazy_static! { + /// flag value cache for disabled_rw + static ref CACHED_disabled_rw: bool = flags_rust::GetServerConfigurableFlag( + "aconfig_flags.aconfig_test", + "com.android.aconfig.test.disabled_rw", + "false") == "true"; + + /// flag value cache for enabled_rw + static ref CACHED_enabled_rw: bool = flags_rust::GetServerConfigurableFlag( + "aconfig_flags.aconfig_test", + "com.android.aconfig.test.enabled_rw", + "true") == "true"; +} + impl FlagProvider { /// query flag disabled_ro pub fn disabled_ro(&self) -> bool { @@ -102,10 +119,7 @@ impl FlagProvider { /// query flag disabled_rw pub fn disabled_rw(&self) -> bool { - flags_rust::GetServerConfigurableFlag( - "aconfig_flags.aconfig_test", - "com.android.aconfig.test.disabled_rw", - "false") == "true" + *CACHED_disabled_rw } /// query flag enabled_fixed_ro @@ -120,10 +134,7 @@ impl FlagProvider { /// query flag enabled_rw pub fn enabled_rw(&self) -> bool { - flags_rust::GetServerConfigurableFlag( - "aconfig_flags.aconfig_test", - "com.android.aconfig.test.enabled_rw", - "true") == "true" + *CACHED_enabled_rw } } diff --git a/tools/aconfig/templates/rust_prod.template b/tools/aconfig/templates/rust_prod.template index e22ad6faa4..30ea646614 100644 --- a/tools/aconfig/templates/rust_prod.template +++ b/tools/aconfig/templates/rust_prod.template @@ -3,16 +3,27 @@ /// flag provider pub struct FlagProvider; +{{ if has_readwrite - }} +lazy_static::lazy_static! \{ + {{ for flag in template_flags }} + {{ if flag.readwrite -}} + /// flag value cache for {flag.name} + static ref CACHED_{flag.name}: bool = flags_rust::GetServerConfigurableFlag( + "aconfig_flags.{flag.device_config_namespace}", + "{flag.device_config_flag}", + "{flag.default_value}") == "true"; + {{ -endif }} + {{ endfor }} +} +{{ -endif }} + impl FlagProvider \{ {{ for flag in template_flags }} /// query flag {flag.name} pub fn {flag.name}(&self) -> bool \{ {{ if flag.readwrite -}} - flags_rust::GetServerConfigurableFlag( - "aconfig_flags.{flag.device_config_namespace}", - "{flag.device_config_flag}", - "{flag.default_value}") == "true" + *CACHED_{flag.name} {{ -else- }} {flag.default_value} {{ -endif }}