[cost] Record zero operation cost for procedures that simply throw

Summary:
Developers complain when a function that used to only throw an exception has complexity increase in the updated revision. Let's suppress such issues by giving those functions 0 cost which is already suppressed by differential reporting.

One common case to the above throw pattern is Java methods that throw an unsupported implementation exception for a functionality that has not been implemented yet. When the developer adds the supported implementation, we don't want to warn them with complexity increase since they are adding new functionality.

This is a design choice/heuristic to prevent noisy results for now.

Reviewed By: skcho

Differential Revision: D25495151

fbshipit-source-id: 94a82b062

infer/src/cost/cost.ml

@@ -363,6 +363,12 @@ let compute_get_node_nb_exec node_cfg bound_map : get_node_nb_exec =
 
 let get_cost_summary ~is_on_ui_thread astate = {CostDomain.post= astate; is_on_ui_thread}
 
+let just_throws_exception proc_desc =
+  Procdesc.get_nodes proc_desc |> List.length <= 5
+  && Procdesc.fold_instrs proc_desc ~init:false ~f:(fun acc _node instr ->
+         match instr with Sil.Store {e1= Lvar pvar; e2= Exn _} -> Pvar.is_return pvar | _ -> acc )
+
+
 let checker ({InterproceduralAnalysis.proc_desc; exe_env; analyze_dependency} as analysis_data) =
   let proc_name = Procdesc.get_proc_name proc_desc in
   let tenv = Exe_env.get_tenv exe_env proc_name in
@@ -432,5 +438,12 @@ let checker ({InterproceduralAnalysis.proc_desc; exe_env; analyze_dependency} as
       (Container.length ~fold:NodeCFG.fold_nodes node_cfg)
       CostDomain.VariantCostMap.pp astate
   in
+  let astate =
+    (* Heuristic: if the original function simply throws an exception,
+       we don't want to report any execution time complexity increase
+       on it to prevent noisy complexity increases. Hence, we set its
+       operation cost to 0 which is filtered in differential mode *)
+    if just_throws_exception proc_desc then CostDomain.set_operation_cost_zero astate else astate
+  in
   Check.check_and_report analysis_data astate ;
   Some (get_cost_summary ~is_on_ui_thread astate)

infer/src/cost/costDomain.ml

@@ -104,6 +104,8 @@ let set_autoreleasepool_size_zero cost_record =
   VariantCostMap.remove CostKind.AutoreleasepoolSize cost_record
 
 
+let set_operation_cost_zero cost_record = VariantCostMap.remove CostKind.OperationCost cost_record
+
 let map ~f cost_record = VariantCostMap.map f cost_record
 
 let find_opt = VariantCostMap.find_opt

infer/src/cost/costDomain.mli

@@ -78,6 +78,8 @@ val get_operation_cost : t -> BasicCostWithReason.t
 
 val set_autoreleasepool_size_zero : t -> t
 
+val set_operation_cost_zero : t -> t
+
 val find_opt : CostKind.t -> t -> BasicCostWithReason.t option
 
 val construct : f:(CostKind.t -> BasicCostWithReason.t) -> t

infer/tests/build_systems/differential_of_costs_report_java/costs_summary.json.exp

@@ -1 +1 @@
-{"top":{"current":2,"previous":1},"unreachable":{"current":1,"previous":0},"zero":{"current":1,"previous":0},"degrees":[{"degree":0,"current":20,"previous":18},{"degree":100,"current":2,"previous":3},{"degree":101,"current":2,"previous":0},{"degree":200,"current":1,"previous":2}]}
+{"top":{"current":2,"previous":1},"unreachable":{"current":1,"previous":0},"zero":{"current":1,"previous":0},"degrees":[{"degree":0,"current":21,"previous":20},{"degree":100,"current":3,"previous":3},{"degree":101,"current":2,"previous":0},{"degree":200,"current":1,"previous":2}]}

infer/tests/build_systems/differential_of_costs_report_java/src/DiffExample.current.java

@@ -88,4 +88,7 @@ public class DiffExample {
         }
     }
 
+  void f10(int x) {
+    f4(x); // don't report here since previous version just throw exception
+  }
 }

infer/tests/build_systems/differential_of_costs_report_java/src/DiffExample.previous.java

@@ -71,4 +71,7 @@ public class DiffExample {
        int x = 0;
    }
 
+  void f10(int x) {
+    throw new IllegalArgumentException("Not implemented yet");
+  }
 }

infer/tests/codetoanalyze/java/performance/UnknownCallsTest.java

@@ -63,6 +63,7 @@ class UnknownCallsTest {
     }
   }
 
+  // functions that just throw have 0 cost to prevent diff reporting
   int throw_exception() {
     throw new IllegalStateException();
   }

infer/tests/codetoanalyze/java/performance/cost-issues.exp

@@ -377,7 +377,7 @@ codetoanalyze/java/performance/UnknownCallsTest.java, UnknownCallsTest.loop_over
 codetoanalyze/java/performance/UnknownCallsTest.java, UnknownCallsTest.may_throw_exception():int, 12,  OnUIThread:false, []
 codetoanalyze/java/performance/UnknownCallsTest.java, UnknownCallsTest.read_max_cost(java.io.InputStream,byte[],int,int,java.util.ArrayList):int, 24 + 5 ⋅ (byteCount + 1),  OnUIThread:false, [{byteCount + 1},Loop]
 codetoanalyze/java/performance/UnknownCallsTest.java, UnknownCallsTest.read_sum_cost(java.io.InputStream,byte[],int,int,java.util.ArrayList):int, 19 + 6 ⋅ 2⋅byteCount,  OnUIThread:false, [{2⋅byteCount},Loop]
-codetoanalyze/java/performance/UnknownCallsTest.java, UnknownCallsTest.throw_exception():int, 5,  OnUIThread:false, []
+codetoanalyze/java/performance/UnknownCallsTest.java, UnknownCallsTest.throw_exception():int, 0,  OnUIThread:false, []
 codetoanalyze/java/performance/UnknownCallsTest.java, UnknownCallsTest.unmodeled_impure_linear(java.util.ArrayList):void, 4 + 13 ⋅ list.length + 3 ⋅ (list.length + 1),  OnUIThread:false, [{list.length + 1},Loop,{list.length},Loop]
 codetoanalyze/java/performance/UnreachableAtExitTest.java, UnreachableAtExitTest.<init>(), 2,  OnUIThread:false, []
 codetoanalyze/java/performance/UnreachableAtExitTest.java, UnreachableAtExitTest.double_prune_unreachable_FN(double):void, 12,  OnUIThread:false, []