From d2b6bc08892a7e2606184fc22bf92cd23311c635 Mon Sep 17 00:00:00 2001 From: Koby Chan Date: Thu, 12 Dec 2024 15:59:15 -0800 Subject: [PATCH] Allow redex to take in a list of baseline configs Summary: Modify redex baseline config parser to take in a list of baseline configs instead of a single config. Reviewed By: jimmycFB Differential Revision: D66314528 fbshipit-source-id: 67e0b085768adf6ec9f26ea5a325153d57bd0abe --- libredex/ConfigFiles.cpp | 133 ++++++++++-------- libredex/ConfigFiles.h | 12 +- .../ArtProfileWriterPass.cpp | 4 +- opt/clinit-outliner/ClinitOutlinePass.cpp | 2 +- opt/interdex/InterDexPass.cpp | 14 +- opt/outliner/MethodSplittingPass.cpp | 2 +- .../OutliningProfileGuidanceImpl.cpp | 2 +- test/integ/BaselineAwareBetamapsTest.config | 32 +++-- 8 files changed, 111 insertions(+), 90 deletions(-) diff --git a/libredex/ConfigFiles.cpp b/libredex/ConfigFiles.cpp index 21c74ee591c..6ad0667b32c 100644 --- a/libredex/ConfigFiles.cpp +++ b/libredex/ConfigFiles.cpp @@ -29,6 +29,7 @@ namespace { constexpr const char* CLASS_MARKER_DELIMITER = "DexEndMarker"; constexpr const char* COLD_START_20PCT_END = "LColdStart20PctEnd"; constexpr const char* COLD_START_1PCT_END = "LColdStart1PctEnd"; +constexpr const char* DEFAULT_BASELINE_PROFILE_CONFIG_NAME = "default"; class StringTabSplitter { private: @@ -574,78 +575,96 @@ bool ConfigFiles::get_did_use_bzl_baseline_profile_config() { } const baseline_profiles::BaselineProfileConfig& -ConfigFiles::get_baseline_profile_config() { - if (m_baseline_profile_config) { - return *m_baseline_profile_config; +ConfigFiles::get_default_baseline_profile_config() { + if (!m_baseline_profile_config_list.empty()) { + auto it = m_baseline_profile_config_list.find( + DEFAULT_BASELINE_PROFILE_CONFIG_NAME); + always_assert(it != m_baseline_profile_config_list.end()); + return it->second; } - m_baseline_profile_config = - std::make_unique(); - Json::Value baseline_profile_config_json; + Json::Value baseline_profile_config_list_json; if (!m_baseline_profile_config_file_name.empty()) { std::ifstream input(m_baseline_profile_config_file_name); Json::Reader reader; - bool parsing_succeeded = reader.parse(input, baseline_profile_config_json); + bool parsing_succeeded = + reader.parse(input, baseline_profile_config_list_json); always_assert_log(parsing_succeeded, "Failed to parse class list json from file: %s\n%s", m_baseline_profile_config_file_name.c_str(), reader.getFormattedErrorMessages().c_str()); } else { - get_json_config().get("baseline_profile", {}, baseline_profile_config_json); + get_json_config().get("baseline_profile", {}, + baseline_profile_config_list_json); } - auto baseline_profile_config_jw = JsonWrapper(baseline_profile_config_json); - - if (baseline_profile_config_json.empty()) { - return *m_baseline_profile_config; - } - - baseline_profile_config_jw.get( - "oxygen_modules", false, - m_baseline_profile_config->options.oxygen_modules); - baseline_profile_config_jw.get( - "strip_classes", false, m_baseline_profile_config->options.strip_classes); - baseline_profile_config_jw.get( - "use_redex_generated_profile", false, - m_baseline_profile_config->options.use_redex_generated_profile); - baseline_profile_config_jw.get( - "include_betamap_20pct_coldstart", true, - m_baseline_profile_config->options.include_betamap_20pct_coldstart); - baseline_profile_config_jw.get( - "betamap_include_coldstart_1pct", false, - m_baseline_profile_config->options.betamap_include_coldstart_1pct); - - auto deepdata_interactions_json = - baseline_profile_config_json.get("deep_data_interaction_config", {}); - always_assert(!deepdata_interactions_json.empty()); - - for (auto it = deepdata_interactions_json.begin(); - it != deepdata_interactions_json.end(); + // Make sure that if list is not empty, it has a default config + always_assert(baseline_profile_config_list_json.empty() || + baseline_profile_config_list_json.isMember( + std::string(DEFAULT_BASELINE_PROFILE_CONFIG_NAME))); + for (auto it = baseline_profile_config_list_json.begin(); + it != baseline_profile_config_list_json.end(); ++it) { - std::string key = it.memberName(); - - const auto& interaction_id = key; - - m_baseline_profile_config->interaction_configs[interaction_id] = {}; - - auto& bpi_config = - m_baseline_profile_config->interaction_configs[interaction_id]; - - const auto& bpi_config_jw = JsonWrapper(*it); - - bpi_config_jw.get("call_threshold", 1, bpi_config.call_threshold); - bpi_config_jw.get("classes", true, bpi_config.classes); - bpi_config_jw.get("post_startup", true, bpi_config.post_startup); - bpi_config_jw.get("startup", false, bpi_config.startup); - bpi_config_jw.get("threshold", 80, bpi_config.threshold); - always_assert(bpi_config_jw.contains("name")); - std::string name = bpi_config_jw.get("name", std::string()); - - m_baseline_profile_config->interactions.emplace_back(interaction_id, - std::move(name)); + std::string config_name = it.memberName(); + const auto& baseline_profile_config_jw = JsonWrapper(*it); + auto current_baseline_profile_config = + baseline_profiles::BaselineProfileConfig(); + baseline_profile_config_jw.get( + "oxygen_modules", false, + current_baseline_profile_config.options.oxygen_modules); + baseline_profile_config_jw.get( + "strip_classes", false, + current_baseline_profile_config.options.strip_classes); + baseline_profile_config_jw.get( + "use_redex_generated_profile", false, + current_baseline_profile_config.options.use_redex_generated_profile); + baseline_profile_config_jw.get("include_betamap_20pct_coldstart", true, + current_baseline_profile_config.options + .include_betamap_20pct_coldstart); + baseline_profile_config_jw.get( + "betamap_include_coldstart_1pct", false, + current_baseline_profile_config.options.betamap_include_coldstart_1pct); + Json::Value deepdata_interactions_json; + baseline_profile_config_jw.get("deep_data_interaction_config", {}, + deepdata_interactions_json); + always_assert(!deepdata_interactions_json.empty()); + for (auto interaction_it = deepdata_interactions_json.begin(); + interaction_it != deepdata_interactions_json.end(); + ++interaction_it) { + std::string key = interaction_it.memberName(); + + const auto& interaction_id = key; + + current_baseline_profile_config.interaction_configs[interaction_id] = {}; + + auto& bpi_config = + current_baseline_profile_config.interaction_configs[interaction_id]; + + const auto& bpi_config_jw = JsonWrapper(*interaction_it); + + bpi_config_jw.get("call_threshold", 1, bpi_config.call_threshold); + bpi_config_jw.get("classes", true, bpi_config.classes); + bpi_config_jw.get("post_startup", true, bpi_config.post_startup); + bpi_config_jw.get("startup", false, bpi_config.startup); + bpi_config_jw.get("threshold", 80, bpi_config.threshold); + always_assert(bpi_config_jw.contains("name")); + std::string name = bpi_config_jw.get("name", std::string()); + + current_baseline_profile_config.interactions.emplace_back( + interaction_id, std::move(name)); + } + m_baseline_profile_config_list.emplace(std::move(config_name), + current_baseline_profile_config); } - return *m_baseline_profile_config; + // Insert a default-constructed config with default values if + // no "default" key was found. Otherwise, this looks up the existing + // value for "default". + auto [it_default, _] = m_baseline_profile_config_list.emplace( + std::string(DEFAULT_BASELINE_PROFILE_CONFIG_NAME), + baseline_profiles::BaselineProfileConfig()); + + return it_default->second; } void ConfigFiles::parse_global_config() { diff --git a/libredex/ConfigFiles.h b/libredex/ConfigFiles.h index 17e0ca0930d..6fac9536f07 100644 --- a/libredex/ConfigFiles.h +++ b/libredex/ConfigFiles.h @@ -14,6 +14,7 @@ #include #include +#include "BaselineProfileConfig.h" #include "GlobalConfig.h" #include "JsonWrapper.h" @@ -33,10 +34,6 @@ namespace api { class AndroidSDK; } // namespace api -namespace baseline_profiles { -struct BaselineProfileConfig; -} // namespace baseline_profiles - namespace inliner { struct InlinerConfig; } // namespace inliner @@ -190,7 +187,8 @@ struct ConfigFiles { /** * Get the global baseline profile config. */ - const baseline_profiles::BaselineProfileConfig& get_baseline_profile_config(); + const baseline_profiles::BaselineProfileConfig& + get_default_baseline_profile_config(); bool get_did_use_bzl_baseline_profile_config(); @@ -268,8 +266,8 @@ struct ConfigFiles { bool m_load_class_lists_attempted{false}; std::unique_ptr m_proguard_map; std::string m_class_frequency_filename; - std::unique_ptr - m_baseline_profile_config; + std::unordered_map + m_baseline_profile_config_list; std::string m_coldstart_class_filename; std::string m_coldstart_methods_filename; std::string m_baseline_profile_config_file_name; diff --git a/opt/art-profile-writer/ArtProfileWriterPass.cpp b/opt/art-profile-writer/ArtProfileWriterPass.cpp index 7641fdcc563..12f7f0abd05 100644 --- a/opt/art-profile-writer/ArtProfileWriterPass.cpp +++ b/opt/art-profile-writer/ArtProfileWriterPass.cpp @@ -548,13 +548,13 @@ void ArtProfileWriterPass::run_pass(DexStoresVector& stores, auto baseline_profile = m_legacy_mode ? get_legacy_baseline_profile() : baseline_profiles::get_baseline_profile( - conf.get_baseline_profile_config(), + conf.get_default_baseline_profile_config(), method_profiles, &method_refs_without_def); auto scope = build_class_scope(stores); if (m_never_compile_callcount_threshold > -1 || m_never_compile_perf_threshold > -1) { never_compile( - scope, conf.get_baseline_profile_config(), method_profiles, + scope, conf.get_default_baseline_profile_config(), method_profiles, m_perf_config.interactions, mgr, m_never_compile_callcount_threshold, m_never_compile_perf_threshold, m_never_compile_excluded_interaction_pattern, diff --git a/opt/clinit-outliner/ClinitOutlinePass.cpp b/opt/clinit-outliner/ClinitOutlinePass.cpp index fa0756c3c80..29a9fe9c04a 100644 --- a/opt/clinit-outliner/ClinitOutlinePass.cpp +++ b/opt/clinit-outliner/ClinitOutlinePass.cpp @@ -31,7 +31,7 @@ void ClinitOutlinePass::run_pass(DexStoresVector& stores, ConfigFiles& conf, PassManager& mgr) { auto& method_profiles = conf.get_method_profiles(); - auto baseline_profile_config = conf.get_baseline_profile_config(); + auto baseline_profile_config = conf.get_default_baseline_profile_config(); if (!m_interaction_pattern.empty()) { boost::regex rx(m_interaction_pattern); std20::erase_if(baseline_profile_config.interaction_configs, diff --git a/opt/interdex/InterDexPass.cpp b/opt/interdex/InterDexPass.cpp index 83fadea670f..bcd989652da 100644 --- a/opt/interdex/InterDexPass.cpp +++ b/opt/interdex/InterDexPass.cpp @@ -181,9 +181,10 @@ void InterDexPass::run_pass( refs_info, &xstore_refs, mgr.get_redex_options().min_sdk, init_classes_with_side_effects, m_transitively_close_interdex_order, m_minimize_cross_dex_refs_explore_alternatives, cache, - m_exclude_baseline_profile_classes, conf.get_baseline_profile_config(), - m_move_coldstart_classes, m_min_betamap_move_threshold, - m_max_betamap_move_threshold, m_stable_partitions); + m_exclude_baseline_profile_classes, + conf.get_default_baseline_profile_config(), m_move_coldstart_classes, + m_min_betamap_move_threshold, m_max_betamap_move_threshold, + m_stable_partitions); if (m_expect_order_list) { always_assert_log( @@ -302,9 +303,10 @@ void InterDexPass::run_pass_on_nonroot_store( refs_info, &xstore_refs, mgr.get_redex_options().min_sdk, init_classes_with_side_effects, m_transitively_close_interdex_order, m_minimize_cross_dex_refs_explore_alternatives, cache, - m_exclude_baseline_profile_classes, conf.get_baseline_profile_config(), - m_move_coldstart_classes, m_min_betamap_move_threshold, - m_max_betamap_move_threshold, m_stable_partitions, + m_exclude_baseline_profile_classes, + conf.get_default_baseline_profile_config(), m_move_coldstart_classes, + m_min_betamap_move_threshold, m_max_betamap_move_threshold, + m_stable_partitions, /* is_root_store */ false); interdex.run_on_nonroot_store(); diff --git a/opt/outliner/MethodSplittingPass.cpp b/opt/outliner/MethodSplittingPass.cpp index 384c0f2c59a..6b14313cd8e 100644 --- a/opt/outliner/MethodSplittingPass.cpp +++ b/opt/outliner/MethodSplittingPass.cpp @@ -78,7 +78,7 @@ void MethodSplittingPass::run_pass(DexStoresVector& stores, size_t reserved_trefs = it == interdex_metrics.end() ? 0 : it->second; auto baseline_profile = baseline_profiles::get_baseline_profile( - conf.get_baseline_profile_config(), conf.get_method_profiles()); + conf.get_default_baseline_profile_config(), conf.get_method_profiles()); InsertOnlyConcurrentSet concurrent_hot_methods; for (auto&& [method, flags] : baseline_profile.methods) { if (flags.hot) { diff --git a/service/method-outliner/OutliningProfileGuidanceImpl.cpp b/service/method-outliner/OutliningProfileGuidanceImpl.cpp index 57b1e690b34..cf01816cae6 100644 --- a/service/method-outliner/OutliningProfileGuidanceImpl.cpp +++ b/service/method-outliner/OutliningProfileGuidanceImpl.cpp @@ -35,7 +35,7 @@ void get_throughput_interactions( boost::regex rx(config.throughput_interaction_name_pattern); for (auto&& [interaction_id, interaction_name] : - config_files.get_baseline_profile_config().interactions) { + config_files.get_default_baseline_profile_config().interactions) { if (boost::regex_match(interaction_name, rx)) { throughput_interaction_ids->insert(interaction_id); auto index = g_redex->get_sb_interaction_index(interaction_id); diff --git a/test/integ/BaselineAwareBetamapsTest.config b/test/integ/BaselineAwareBetamapsTest.config index 3ac75fac897..a0e04e6b4a5 100644 --- a/test/integ/BaselineAwareBetamapsTest.config +++ b/test/integ/BaselineAwareBetamapsTest.config @@ -5,22 +5,24 @@ ], }, "baseline_profile": { - "deep_data_interaction_config": { - "ColdStart": { - "name": "ColdStart", - "call_threshold": 1, - "classes": true, - "post_startup": true, - "startup": true, - "threshold": 20 + "default": { + "deep_data_interaction_config": { + "ColdStart": { + "name": "ColdStart", + "call_threshold": 1, + "classes": true, + "post_startup": true, + "startup": true, + "threshold": 20 + }, + "12345678": { + "name": "Other", + }, }, - "12345678": { - "name": "Other", - }, - }, - "oxygen_modules": true, - "strip_classes": false, - "use_redex_generated_profile": false, + "oxygen_modules": true, + "strip_classes": false, + "use_redex_generated_profile": false, + } }, "InterDexPass": { "static_prune": false,