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
facebook · Sep 13, 2023 · 96bc1df · 96bc1df
1 parent 0da5459
commit 96bc1df
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 65 deletions.
diff --git 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
@@ -133,38 +133,19 @@ void ClassMergingPass::bind_config() {
   std::vector<Json::Value> 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) {
       m_max_num_dispatch_target =
           boost::optional<size_t>(static_cast<size_t>(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
@@ -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
@@ -184,24 +184,25 @@ std::vector<ConstTypeHashSet>& 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<ConstTypeHashSet>(num_group);
   if (num_group == 1) {
     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<std::string, InterDexGroupingType>
       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
@@ -29,28 +29,34 @@ 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<ConstTypeHashSet>& group_by_interdex_set(
       const Scope& scope, const ConstTypeHashSet& types);
 
   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<ConstTypeHashSet> m_all_interdexing_groups;
 };
 

diff --git 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
@@ -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; }