Promote "Use Stats for excludable_kotlin_lambda metric"

Summary: Promote {D90710396}.

Reviewed By: thezhangwei

Differential Revision: D91087950

fbshipit-source-id: 5f011e8891408e049a81a153ef6bfb843151ca16

Author: xuhdev

opt/kotlin-lambda/KotlinInstanceRewriter.cpp

@@ -6,9 +6,11 @@
  */
 
 #include "KotlinInstanceRewriter.h"
+#include "AtomicStatCounter.h"
 #include "CFGMutation.h"
 #include "PassManager.h"
 #include "Show.h"
+#include "TypeUtil.h"
 #include "Walkers.h"
 
 namespace {
@@ -29,12 +31,14 @@ KotlinInstanceRewriter::Stats KotlinInstanceRewriter::collect_instance_usage(
                   std::set<std::pair<IRInstruction*, DexMethod*>>>&
         concurrent_instance_map,
 
-    std::function<bool(DexClass*)> do_not_consider_type) {
+    std::function<bool(DexClass*)> is_excludable,
+    bool exclude_excludable) {
   // Collect all the types which are of Kotlin classes which sets INSTANCE
   // variable.
   // Get all the uses of the INSTANCE variables whose <init> does not have
   // sideeffets
   KotlinInstanceRewriter::Stats stats{};
+  AtomicStatCounter<size_t> excludable_count{0};
   walk::parallel::classes(scope, [&](DexClass* cls) {
     if (!can_rename(cls) || !can_delete(cls)) {
       return;
@@ -43,16 +47,24 @@ KotlinInstanceRewriter::Stats KotlinInstanceRewriter::collect_instance_usage(
     if (instance == nullptr) {
       return;
     }
-    if (do_not_consider_type(cls)) {
+    if (!type::is_kotlin_non_capturing_lambda(cls)) {
       return;
     }
+    // Count excludable classes (e.g., hot lambdas)
+    if (is_excludable(cls)) {
+      excludable_count++;
+      if (exclude_excludable) {
+        return;
+      }
+    }
     if (concurrent_instance_map.count(instance) != 0u) {
       return;
     }
     std::set<std::pair<IRInstruction*, DexMethod*>> insns;
     concurrent_instance_map.emplace(instance, insns);
   });
   stats.kotlin_new_instance = concurrent_instance_map.size();
+  stats.excludable_kotlin_lambda = excludable_count;
   return stats;
 }
 
@@ -219,6 +231,7 @@ void KotlinInstanceRewriter::Stats::report(PassManager& mgr) const {
   mgr.incr_metric("kotlin_instance_fields_removed",
                   kotlin_instance_fields_removed);
   mgr.incr_metric("kotlin_new_inserted", kotlin_new_inserted);
+  mgr.incr_metric("excludable_kotlin_lambda", excludable_kotlin_lambda);
 
   TRACE(KOTLIN_INSTANCE, 1, "kotlin_new_instance = %zu", kotlin_new_instance);
   TRACE(KOTLIN_INSTANCE, 1, "kotlin_new_instance_which_escapes = %zu",
@@ -228,4 +241,6 @@ void KotlinInstanceRewriter::Stats::report(PassManager& mgr) const {
   TRACE(KOTLIN_INSTANCE, 1, "kotlin_instance_fields_removed = %zu",
         kotlin_instance_fields_removed);
   TRACE(KOTLIN_INSTANCE, 1, "kotlin_new_inserted = %zu", kotlin_new_inserted);
+  TRACE(KOTLIN_INSTANCE, 1, "excludable_kotlin_lambda = %zu",
+        excludable_kotlin_lambda);
 }

opt/kotlin-lambda/KotlinInstanceRewriter.h

@@ -20,13 +20,15 @@ class KotlinInstanceRewriter {
     size_t kotlin_instances_with_single_use{0};
     size_t kotlin_instance_fields_removed{0};
     size_t kotlin_new_inserted{0};
+    size_t excludable_kotlin_lambda{0};
     Stats& operator+=(const Stats& that) {
       kotlin_new_instance += that.kotlin_new_instance;
       kotlin_new_instance_which_escapes +=
           that.kotlin_new_instance_which_escapes;
       kotlin_instances_with_single_use += that.kotlin_instances_with_single_use;
       kotlin_instance_fields_removed += that.kotlin_instance_fields_removed;
       kotlin_new_inserted += that.kotlin_new_inserted;
+      excludable_kotlin_lambda += that.excludable_kotlin_lambda;
       return *this;
     }
     // Updates metrics tracked by \p mgr corresponding to these statistics.
@@ -43,12 +45,15 @@ class KotlinInstanceRewriter {
   // the same type and is initilized in <clinit>. Collect all such Lambda. Map
   // contains the field (that contains INSTANCE) and {insn, method} where it is
   // read (or used).
+  // - is_excludable: returns true if the class is excludable (e.g., hot lambda)
+  // - exclude_excludable: if true, excludable classes are skipped
   Stats collect_instance_usage(
       const Scope& scope,
       ConcurrentMap<DexFieldRef*,
                     std::set<std::pair<IRInstruction*, DexMethod*>>>&
           concurrent_instance_map,
-      std::function<bool(DexClass*)> do_not_consider_type);
+      std::function<bool(DexClass*)> is_excludable,
+      bool exclude_excludable);
 
   // Filter out any INSTANCE that might escape and whose use we might not be
   // able to track

opt/kotlin-lambda/KotlinStatelessLambdaSingletonRemovalPass.cpp

@@ -7,22 +7,12 @@
 
 #include "KotlinStatelessLambdaSingletonRemovalPass.h"
 
-#include "AtomicStatCounter.h"
 #include "ConfigFiles.h"
 #include "DexUtil.h"
 #include "KotlinInstanceRewriter.h"
 #include "MethodProfiles.h"
 #include "PassManager.h"
 #include "TypeUtil.h"
-#include "Walkers.h"
-
-namespace {
-
-bool is_kotlin_stateless_lambda(const DexClass* cls) {
-  return type::is_kotlin_non_capturing_lambda(cls);
-}
-
-} // namespace
 
 bool KotlinStatelessLambdaSingletonRemovalPass::is_hot_lambda(
     const DexClass* cls,
@@ -54,34 +44,18 @@ void KotlinStatelessLambdaSingletonRemovalPass::run_pass(
   const auto& method_profiles = conf.get_method_profiles();
   const bool has_method_profiles = method_profiles.has_stats();
 
-  // Count hot non-capturing lambdas for stats reporting
-  AtomicStatCounter<size_t> excludable_stateless_kotlin_lambda{0};
-  if (has_method_profiles) {
-    walk::parallel::classes(scope, [&](DexClass* cls) {
-      if (is_kotlin_stateless_lambda(cls) &&
-          is_hot_lambda(cls, method_profiles)) {
-        excludable_stateless_kotlin_lambda++;
-      }
-    });
-  }
-
-  auto do_not_consider_type =
+  auto is_excludable =
       [this, &method_profiles, has_method_profiles](DexClass* cls) -> bool {
-    return !is_kotlin_stateless_lambda(cls) ||
-           (m_exclude_hot && has_method_profiles &&
-            is_hot_lambda(cls, method_profiles));
+    redex_assert(type::is_kotlin_non_capturing_lambda(cls));
+    return has_method_profiles && is_hot_lambda(cls, method_profiles);
   };
 
-  // TODO: Update the rewriter logic
   KotlinInstanceRewriter::Stats stats = rewriter.collect_instance_usage(
-      scope, concurrentLambdaMap, do_not_consider_type);
+      scope, concurrentLambdaMap, is_excludable, m_exclude_hot);
 
   stats += rewriter.remove_escaping_instance(scope, concurrentLambdaMap);
   stats += rewriter.transform(concurrentLambdaMap);
   stats.report(mgr);
-
-  mgr.incr_metric("excludable_stateless_kotlin_lambda",
-                  excludable_stateless_kotlin_lambda);
 }
 
 static KotlinStatelessLambdaSingletonRemovalPass s_pass;