From 96bc1df3472fe15d773d022b0d936740a32c78b0 Mon Sep 17 00:00:00 2001 From: "Wei Zhang (Devinfra)" Date: Tue, 12 Sep 2023 17:48:03 -0700 Subject: [PATCH] Add InterDexGroupingConfig Summary: - Add `InterDexGroupingConfig` to abstract the type and inferring_mode configs - Simplify the InterDexGrouping refig options - `InterDexGroupingType` is "DISABLED" by default; `InterDexGroupingInferringMode` is "class-loads" by default - Class Merging passes initial config values: - ClassMergingPass `type`: `DISABLED` by default; override per refig models - ClassMergingPass `inferring_mode`: "class-loads" by default; override to "class-loads-bb" in refig - AnonymousClassMerging + IntraDexClassMerging `type`: `NON_ORDERED_SET`; no override in refig - AnonymousClassMerging + IntraDexClassMerging `inferring_mode`: no override, fallback to the "class-loads" default Reviewed By: beicy Differential Revision: D48923479 fbshipit-source-id: 14299411faa4ed9b727bf95f3d1ccfb3b1c5d7de --- .../AnonymousClassMergingPass.cpp | 4 +- opt/class-merging/ClassMergingPass.cpp | 42 +++++-------------- .../IntraDexClassMergingPass.cpp | 4 +- service/class-merging/InterDexGrouping.cpp | 31 ++++++++++---- service/class-merging/InterDexGrouping.h | 28 ++++++++----- service/class-merging/Model.cpp | 4 +- service/class-merging/Model.h | 9 +--- 7 files changed, 57 insertions(+), 65 deletions(-) diff --git a/opt/class-merging/AnonymousClassMergingPass.cpp b/opt/class-merging/AnonymousClassMergingPass.cpp index 2a2058af4b1..3a39eea4cff 100644 --- a/opt/class-merging/AnonymousClassMergingPass.cpp +++ b/opt/class-merging/AnonymousClassMergingPass.cpp @@ -43,8 +43,8 @@ void AnonymousClassMergingPass::bind_config() { "Maximum mergeable class count per merging group"); std::string interdex_grouping; bind("interdex_grouping", "non-ordered-set", interdex_grouping); - m_merging_spec.interdex_grouping = - InterDexGrouping::get_merge_per_interdex_type(interdex_grouping); + // Inferring_mode is "class-loads" by default. + m_merging_spec.interdex_config.init_type(interdex_grouping); } void AnonymousClassMergingPass::run_pass(DexStoresVector& stores, diff --git a/opt/class-merging/ClassMergingPass.cpp b/opt/class-merging/ClassMergingPass.cpp index aefe1a60f40..281c131bca5 100644 --- a/opt/class-merging/ClassMergingPass.cpp +++ b/opt/class-merging/ClassMergingPass.cpp @@ -133,9 +133,9 @@ void ClassMergingPass::bind_config() { std::vector models; bind("models", {}, models); - std::string dflt_interdex_grouping_inferring_mode; - bind("default_interdex_grouping_inferring_mode", "", - dflt_interdex_grouping_inferring_mode); + std::string interdex_grouping_inferring_mode; + bind("interdex_grouping_inferring_mode", "class-loads", + interdex_grouping_inferring_mode); after_configuration([=] { if (max_num_dispatch_target > 0) { @@ -143,28 +143,9 @@ void ClassMergingPass::bind_config() { boost::optional(static_cast(max_num_dispatch_target)); } - if (models.empty()) return; - - auto parse_grouping_inferring_mode = [](const std::string& s, - InterDexGroupingInferringMode - dflt) { - if (s.empty()) { - return dflt; - } - if (s == "all-types") { - return InterDexGroupingInferringMode::kAllTypeRefs; - } else if (s == "class-loads") { - return InterDexGroupingInferringMode::kClassLoads; - } else if (s == "class-loads-bb") { - return InterDexGroupingInferringMode::kClassLoadsBasicBlockFiltering; - } else { - always_assert_log(false, "Unknown interdex-grouping-inferring-mode %s", - s.c_str()); - } - }; - InterDexGroupingInferringMode default_mode = parse_grouping_inferring_mode( - dflt_interdex_grouping_inferring_mode, - ModelSpec().interdex_grouping_inferring_mode); + if (models.empty()) { + return; + } // load each model spec for erasure for (auto it = models.begin(); it != models.end(); ++it) { @@ -224,10 +205,9 @@ void ClassMergingPass::bind_config() { // InterDex grouping option is by default `non-ordered-set`. std::string interdex_grouping; model_spec.get("interdex_grouping", "non-ordered-set", interdex_grouping); - model.interdex_grouping = - InterDexGrouping::get_merge_per_interdex_type(interdex_grouping); + model.interdex_config.init_type(interdex_grouping); - always_assert_log(!model.interdex_grouping || + always_assert_log(!model.interdex_config.is_enabled() || (model.type_tag_config != TypeTagConfig::NONE), "Cannot group %s when type tag is not needed.", model.name.c_str()); @@ -262,10 +242,8 @@ void ClassMergingPass::bind_config() { // Assume config based models are all generated code. model_spec.get("is_generated_code", true, model.is_generated_code); - std::string usage_mode_str = - model_spec.get("type_usage_mode", std::string("")); - model.interdex_grouping_inferring_mode = - parse_grouping_inferring_mode(usage_mode_str, default_mode); + model.interdex_config.init_inferring_mode( + interdex_grouping_inferring_mode); if (!verify_model_spec(m_model_specs, model)) { continue; diff --git a/opt/class-merging/IntraDexClassMergingPass.cpp b/opt/class-merging/IntraDexClassMergingPass.cpp index 8cb0eefb8ba..fda4432249b 100644 --- a/opt/class-merging/IntraDexClassMergingPass.cpp +++ b/opt/class-merging/IntraDexClassMergingPass.cpp @@ -42,8 +42,8 @@ void IntraDexClassMergingPass::bind_config() { } std::string interdex_grouping; bind("interdex_grouping", "non-ordered-set", interdex_grouping); - m_merging_spec.interdex_grouping = - InterDexGrouping::get_merge_per_interdex_type(interdex_grouping); + // Inferring_mode is "class-loads" by default. + m_merging_spec.interdex_config.init_type(interdex_grouping); } void IntraDexClassMergingPass::run_pass(DexStoresVector& stores, diff --git a/service/class-merging/InterDexGrouping.cpp b/service/class-merging/InterDexGrouping.cpp index 526ffca1b65..d06e97b3397 100644 --- a/service/class-merging/InterDexGrouping.cpp +++ b/service/class-merging/InterDexGrouping.cpp @@ -184,7 +184,7 @@ std::vector& InterDexGrouping::group_by_interdex_set( TRACE(CLMG, 5, "num_interdex_groups %zu; cls_to_interdex_groups %zu", num_interdex_groups, cls_to_interdex_groups.size()); size_t num_group = 1; - if (is_enabled() && num_interdex_groups > 1) { + if (m_config.is_enabled() && num_interdex_groups > 1) { num_group = num_interdex_groups; } m_all_interdexing_groups = std::vector(num_group); @@ -192,16 +192,17 @@ std::vector& InterDexGrouping::group_by_interdex_set( m_all_interdexing_groups[0].insert(types.begin(), types.end()); return m_all_interdexing_groups; } - const auto& type_to_usages = get_type_usages(types, scope, m_inferring_mode); + const auto& type_to_usages = + get_type_usages(types, scope, m_config.inferring_mode); for (const auto& pair : type_to_usages) { auto index = get_interdex_group(pair.second, cls_to_interdex_groups, num_interdex_groups); - if (m_type == InterDexGroupingType::NON_HOT_SET) { + if (m_config.type == InterDexGroupingType::NON_HOT_SET) { if (index == 0) { // Drop mergeables that are in the hot set. continue; } - } else if (m_type == InterDexGroupingType::NON_ORDERED_SET) { + } else if (m_config.type == InterDexGroupingType::NON_ORDERED_SET) { if (index < num_interdex_groups - 1) { // Only merge the last group which are not in ordered set, drop other // mergeables. @@ -225,8 +226,7 @@ TypeSet InterDexGrouping::get_types_in_current_interdex_group( return group; } -InterDexGroupingType InterDexGrouping::get_merge_per_interdex_type( - const std::string& interdex_grouping) { +void InterDexGroupingConfig::init_type(const std::string& interdex_grouping) { const static std::unordered_map string_to_grouping = { @@ -239,7 +239,24 @@ InterDexGroupingType InterDexGrouping::get_merge_per_interdex_type( "InterDex Grouping Type %s not found. Please check the list" " of accepted values.", interdex_grouping.c_str()); - return string_to_grouping.at(interdex_grouping); + this->type = string_to_grouping.at(interdex_grouping); +} + +void InterDexGroupingConfig::init_inferring_mode(const std::string& mode) { + if (mode.empty()) { + this->inferring_mode = InterDexGroupingInferringMode::kAllTypeRefs; + } + if (mode == "all-types") { + this->inferring_mode = InterDexGroupingInferringMode::kAllTypeRefs; + } else if (mode == "class-loads") { + this->inferring_mode = InterDexGroupingInferringMode::kClassLoads; + } else if (mode == "class-loads-bb") { + this->inferring_mode = + InterDexGroupingInferringMode::kClassLoadsBasicBlockFiltering; + } else { + always_assert_log(false, "Unknown interdex-grouping-inferring-mode %s", + mode.c_str()); + } } std::ostream& operator<<(std::ostream& os, InterDexGroupingInferringMode mode) { diff --git a/service/class-merging/InterDexGrouping.h b/service/class-merging/InterDexGrouping.h index f42cd4da716..d724894fb78 100644 --- a/service/class-merging/InterDexGrouping.h +++ b/service/class-merging/InterDexGrouping.h @@ -29,14 +29,24 @@ enum class InterDexGroupingInferringMode { kClassLoadsBasicBlockFiltering, }; +struct InterDexGroupingConfig { + explicit InterDexGroupingConfig(InterDexGroupingType type) + : type(type), + inferring_mode(InterDexGroupingInferringMode::kClassLoads) {} + + InterDexGroupingType type; + InterDexGroupingInferringMode inferring_mode; + + bool is_enabled() const { return type != DISABLED; } + + void init_type(const std::string& interdex_grouping); + void init_inferring_mode(const std::string& mode); +}; + class InterDexGrouping final { public: - explicit InterDexGrouping(ConfigFiles& conf, - InterDexGroupingType type, - InterDexGroupingInferringMode mode) - : m_conf(conf), m_type(type), m_inferring_mode(mode) {} - - bool is_enabled() const { return m_type != InterDexGroupingType::DISABLED; } + explicit InterDexGrouping(ConfigFiles& conf, InterDexGroupingConfig config) + : m_conf(conf), m_config(config) {} std::vector& group_by_interdex_set( const Scope& scope, const ConstTypeHashSet& types); @@ -44,13 +54,9 @@ class InterDexGrouping final { TypeSet get_types_in_current_interdex_group( const TypeSet& types, const ConstTypeHashSet& interdex_group_types); - static InterDexGroupingType get_merge_per_interdex_type( - const std::string& interdex_grouping); - private: ConfigFiles& m_conf; - const InterDexGroupingType m_type; - const InterDexGroupingInferringMode m_inferring_mode; + const InterDexGroupingConfig m_config; std::vector m_all_interdexing_groups; }; diff --git a/service/class-merging/Model.cpp b/service/class-merging/Model.cpp index d57eb07732c..88f7354bfb2 100644 --- a/service/class-merging/Model.cpp +++ b/service/class-merging/Model.cpp @@ -593,9 +593,7 @@ void Model::flatten_shapes(const MergerType& merger, MergerType::ShapeCollector& shapes) { size_t num_trimmed_types = trim_groups(shapes, m_spec.min_count); m_stats.m_dropped += num_trimmed_types; - // Group all merging targets according to interdex grouping. - InterDexGrouping interdex_grouping(m_conf, m_spec.interdex_grouping, - m_spec.interdex_grouping_inferring_mode); + InterDexGrouping interdex_grouping(m_conf, m_spec.interdex_config); const auto& all_interdex_groups = interdex_grouping.group_by_interdex_set(m_scope, m_spec.merging_targets); // sort shapes by mergeables count diff --git a/service/class-merging/Model.h b/service/class-merging/Model.h index 33fceb79cd4..b8aad8274d5 100644 --- a/service/class-merging/Model.h +++ b/service/class-merging/Model.h @@ -140,7 +140,7 @@ struct ModelSpec { strategy::Strategy strategy{strategy::BY_CLASS_COUNT}; // Group splitting. This is looser than the per dex split and takes into // account the interdex order (if any provided). - InterDexGroupingType interdex_grouping{InterDexGroupingType::DISABLED}; + InterDexGroupingConfig interdex_config{InterDexGroupingType::DISABLED}; // whether to perform class merging on the primary dex. bool include_primary_dex{false}; // Process @MethodMeta annotations @@ -166,9 +166,6 @@ struct ModelSpec { // a part of the generated set. bool is_generated_code{false}; - InterDexGroupingInferringMode interdex_grouping_inferring_mode{ - InterDexGroupingInferringMode::kAllTypeRefs}; - bool generate_type_tag() const { return type_tag_config == TypeTagConfig::GENERATE; } @@ -281,10 +278,6 @@ class Model { return m_spec.class_name_prefix; } - bool is_interdex_grouping_enabled() const { - return m_spec.interdex_grouping != InterDexGroupingType::DISABLED; - } - const ModelSpec& get_model_spec() const { return m_spec; } const ModelStats& get_model_stats() const { return m_stats; }