Track use of reflected types in mergeability checks

Summary: # Context As more and more code has been migrated to Kotlin, more anonymous classes emerged in the order set in the betamap. Since we inject artificial `const-class` instructions in class preload skeleton methods for the big apps in `ClassPreloadGenerator`, we ended up marking those startup set classes as reflected. As a result, we are excluding a larger number of classes as not mergeable. # The Change If we actually track the use of the `const-class`es using `DefUseChains`, we will be able to identify the artificial ones and not register them as real reflections. Reviewed By: ssj933 Differential Revision: D55993834 fbshipit-source-id: ea440ecd17e3f2741503ef0bd3ed4d0fb8221850
joychic · Apr 16, 2024 · 7b5efcb · 7b5efcb
1 parent 8f3e60f
commit 7b5efcb
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 12 deletions.
diff --git a/opt/class-merging/IntraDexClassMergingPass.cpp b/opt/class-merging/IntraDexClassMergingPass.cpp
@@ -41,6 +41,8 @@ void IntraDexClassMergingPass::bind_config() {
     m_merging_spec.max_count = boost::optional<size_t>(max_count);
   }
   bind("use_stable_shape_names", false, m_merging_spec.use_stable_shape_names);
+  bind("mergeability_checks_use_of_const_class", false,
+       m_merging_spec.mergeability_checks_use_of_const_class);
   std::string interdex_grouping;
   bind("interdex_grouping", "non-ordered-set", interdex_grouping);
   m_merging_spec.interdex_config.init_type(interdex_grouping);

diff --git a/opt/class-merging/ModelSpecGenerator.cpp b/opt/class-merging/ModelSpecGenerator.cpp
@@ -6,7 +6,9 @@
  */
 
 #include "ModelSpecGenerator.h"
+
 #include "ClassMerging.h"
+#include "LiveRange.h"
 #include "Model.h"
 #include "PassManager.h"
 #include "ReflectionAnalysis.h"
@@ -77,6 +79,9 @@ TypeSet collect_reflected_mergeables(
   }
 
   auto& cfg = code->cfg();
+  live_range::MoveAwareChains chains(cfg);
+  live_range::DefUseChains du_chains = chains.get_def_use_chains();
+
   for (const auto& mie : cfg::InstructionIterable(cfg)) {
     auto insn = mie.insn;
     auto aobj = analysis->get_result_abstract_object(insn);
@@ -86,12 +91,21 @@ TypeSet collect_reflected_mergeables(
       reflected_type = const_cast<DexType*>(
           type::get_element_type_if_array(aobj->get_dex_type()));
     }
-    if (reflected_type != nullptr &&
-        merging_spec->merging_targets.count(reflected_type) > 0) {
-      non_mergeables.insert(reflected_type);
-      TRACE(CLMG, 5, "[reflected mergeable] %s (%s) in %s", SHOW(insn),
-            SHOW(reflected_type), SHOW(method));
+    if (reflected_type == nullptr ||
+        merging_spec->merging_targets.count(reflected_type) == 0) {
+      continue;
+    }
+    const auto use_set = du_chains[insn];
+    if (merging_spec->mergeability_checks_use_of_const_class &&
+        use_set.empty()) {
+      TRACE(CLMG, 5, "[reflected mergeable] skipped w/o no use %s in %s",
+            SHOW(insn), SHOW(method));
+      continue;
     }
+
+    non_mergeables.insert(reflected_type);
+    TRACE(CLMG, 5, "[reflected mergeable] %s (%s) in %s", SHOW(insn),
+          SHOW(reflected_type), SHOW(method));
   }
 
   return non_mergeables;

diff --git a/service/class-merging/MergeabilityCheck.cpp b/service/class-merging/MergeabilityCheck.cpp
@@ -27,7 +27,8 @@ MergeabilityChecker::MergeabilityChecker(const Scope& scope,
       m_spec(spec),
       m_ref_checker(ref_checker),
       m_generated(generated),
-      m_const_class_safe_types(spec.const_class_safe_types) {}
+      m_const_class_safe_types(spec.const_class_safe_types),
+      m_track_use_of_reflection(spec.mergeability_checks_use_of_const_class) {}
 
 void MergeabilityChecker::exclude_unsupported_cls_property(
     TypeSet& non_mergeables) {
@@ -151,13 +152,29 @@ TypeSet MergeabilityChecker::exclude_unsupported_bytecode_refs_for(
 
     const auto* type = type::get_element_type_if_array(insn->get_type());
     if (m_spec.merging_targets.count(type) > 0) {
-      if (!opcode::is_const_class(insn->opcode()) ||
-          m_const_class_safe_types.empty()) {
-        non_mergeables.insert(type);
+      if (m_track_use_of_reflection) {
+        // Always track the const-class usages.
+        if (!opcode::is_const_class(insn->opcode())) {
+          TRACE(CLMG, 5, "[non mergeable] unsafe opcode @ %s in %s", SHOW(insn),
+                SHOW(method));
+          non_mergeables.insert(type);
+        } else {
+          // To verify the usages
+          const_classes_to_verify.emplace_back(insn, type);
+          const_classes_to_verify_set.insert(insn);
+        }
       } else {
-        // To verify the usages
-        const_classes_to_verify.emplace_back(insn, type);
-        const_classes_to_verify_set.insert(insn);
+        // Old logic that also checks m_const_class_safe_types not being empty.
+        if (!opcode::is_const_class(insn->opcode()) ||
+            m_const_class_safe_types.empty()) {
+          TRACE(CLMG, 5, "[non mergeable] unsafe opcode @ %s in %s", SHOW(insn),
+                SHOW(method));
+          non_mergeables.insert(type);
+        } else {
+          // To verify the usages
+          const_classes_to_verify.emplace_back(insn, type);
+          const_classes_to_verify_set.insert(insn);
+        }
       }
     }
   }

diff --git a/service/class-merging/MergeabilityCheck.h b/service/class-merging/MergeabilityCheck.h
@@ -40,6 +40,7 @@ class MergeabilityChecker {
   const RefChecker& m_ref_checker;
   const TypeSet& m_generated;
   const std::unordered_set<DexType*>& m_const_class_safe_types;
+  bool m_track_use_of_reflection;
 
   void exclude_unsupported_cls_property(TypeSet& non_mergeables);
   void exclude_unsupported_bytecode(TypeSet& non_mergeables);

diff --git a/service/class-merging/Model.h b/service/class-merging/Model.h
@@ -196,6 +196,8 @@ struct ModelSpec {
   boost::optional<size_t> max_num_dispatch_target{boost::none};
 
   bool use_stable_shape_names{false};
+
+  bool mergeability_checks_use_of_const_class{false};
 };
 
 struct ModelStats {