From d712635feb9e2b516cf44cedb22e51079e18920b Mon Sep 17 00:00:00 2001
From: jrm <jrm@fb.com>
Date: Wed, 25 Nov 2015 21:52:16 -0800
Subject: [PATCH] Automatically infer the @PerformanceCritical annotations from
 the overriden methods

Summary: public
This allows to run the checker and get feedback about potential expensive call stacks without having to annotate first all the methods that are overriding PerofrmanceCritical-annotated methods

Reviewed By: cristianoc

Differential Revision: D2693556

fb-gh-sync-id: cb60278
---
 infer/src/backend/callTree.ml                 | 13 +++++++++
 infer/src/backend/callTree.mli                |  3 +++
 infer/src/checkers/performanceCritical.ml     | 23 ++++++++++++++--
 .../java/checkers/ExpensiveCallExample.java   | 27 +++++++++++++++++--
 .../java/checkers/ExpensiveCallTest.java      |  3 ++-
 5 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/infer/src/backend/callTree.ml b/infer/src/backend/callTree.ml
index c1e030ffe..2c9b1e4e1 100644
--- a/infer/src/backend/callTree.ml
+++ b/infer/src/backend/callTree.ml
@@ -8,6 +8,19 @@
  *)
 
 
+module F = Format
+
+
 type t =
   | Direct of Procname.t
   | Indirect of Procname.t * t list
+
+
+let pp fmt tree =
+  let rec loop stack = function
+    | Direct pname ->
+        F.fprintf fmt "%s -> %s" stack (Procname.to_string pname)
+    | Indirect (pname, l) ->
+        let stack' = stack ^ " -> " ^ (Procname.to_string pname) in
+        IList.iter (loop stack') l in
+  loop "@" tree
diff --git a/infer/src/backend/callTree.mli b/infer/src/backend/callTree.mli
index c58f9105f..da91fdb87 100644
--- a/infer/src/backend/callTree.mli
+++ b/infer/src/backend/callTree.mli
@@ -11,3 +11,6 @@
 type t =
   | Direct of Procname.t
   | Indirect of Procname.t * t list
+
+(** print the list of call stacks in the tree *)
+val pp : Format.formatter -> t -> unit
diff --git a/infer/src/checkers/performanceCritical.ml b/infer/src/checkers/performanceCritical.ml
index e4aaeec74..8219e497d 100644
--- a/infer/src/checkers/performanceCritical.ml
+++ b/infer/src/checkers/performanceCritical.ml
@@ -10,6 +10,9 @@
 module L = Logging
 
 
+let infer_performance_critical_methods = true
+
+
 let calls_expensive_method =
   "CHECKERS_CALLS_EXPENSIVE_METHOD"
 
@@ -45,6 +48,17 @@ let method_is_performance_critical pname =
   check_method Annotations.ia_is_performance_critical pname
 
 
+let method_overrides_performance_critical tenv pname =
+  let overrides () =
+    let found = ref false in
+    PatternMatch.proc_iter_overridden_methods
+      (fun pn -> found := method_is_performance_critical pn)
+      tenv pname;
+    !found in
+  method_is_performance_critical pname
+  || overrides ()
+
+
 let method_is_expensive pname =
   check_method Annotations.ia_is_expensive pname
 
@@ -152,12 +166,17 @@ let check_one_procedure tenv pname pdesc =
     let checked_pnames = ref Procname.Set.empty in
     Cfg.Procdesc.fold_calls
       (collect_expensive_call pdesc checked_pnames) [] pdesc in
+
+  update_summary expensive_call_trees pname;
+
   match expensive_call_trees with
   | [] -> ()
+  | call_trees when infer_performance_critical_methods ->
+      if method_overrides_performance_critical tenv pname then
+        report_expensive_calls pname pdesc loc call_trees
   | call_trees when performance_critical ->
       report_expensive_calls pname pdesc loc call_trees
-  | call_trees ->
-      update_summary call_trees pname
+  | _ -> ()
 
 
 let callback_performance_checker _ get_proc_desc _ tenv pname proc_desc =
diff --git a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java
index 1713a5249..9268c93f3 100644
--- a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java
+++ b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java
@@ -12,15 +12,29 @@ package codetoanalyze.java.checkers;
 import com.facebook.infer.annotation.Expensive;
 import com.facebook.infer.annotation.PerformanceCritical;
 
+
+class Other {
+
+  @Expensive
+  void expensive() {
+  }
+
+  void callsExpensive1() {
+    expensive();
+  }
+
+}
+
+
 public class ExpensiveCallExample {
 
-  Object mObject;
+  Other mOther;
 
   void nonExpensiveMethod() {}
 
   @Expensive
   void expensiveMethod() {
-    mObject = new Object();
+    mOther = new Other();
   }
 
   void methodWrapper() {
@@ -47,4 +61,13 @@ public class ExpensiveCallExample {
     object.m5();
   }
 
+  void callsExpensive2() {
+    mOther.callsExpensive1();
+  }
+
+  @PerformanceCritical
+  void longerCallStackToExpensive() {
+    callsExpensive2();
+  }
+
 }
diff --git a/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java b/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java
index 3b5a6b93d..dfb6b92ed 100644
--- a/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java
+++ b/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java
@@ -33,7 +33,7 @@ public class ExpensiveCallTest {
   @BeforeClass
   public static void loadResults() throws InterruptedException, IOException {
     inferResults =
-      InferResults.loadCheckersResults(ImmutableCastTest.class, SOURCE_FILE);
+      InferResults.loadCheckersResults(ExpensiveCallTest.class, SOURCE_FILE);
   }
 
   @Test
@@ -43,6 +43,7 @@ public class ExpensiveCallTest {
         "directlyCallingExpensiveMethod",
         "indirectlyCallingExpensiveMethod",
         "callingExpensiveMethodFromInterface",
+        "longerCallStackToExpensive",
     };
     assertThat(
         "Results should contain " + CALLS_EXPENSIVE_METHOD,