[cost] Invoke Cost issues only once

Reviewed By: mbouaziz

Differential Revision: D15853454

fbshipit-source-id: 41ec36392

infer/src/backend/Differential.ml

@@ -79,10 +79,10 @@ module CostsSummary = struct
     in
     List.fold ~init
       ~f:(fun acc (v : Jsonbug_t.cost_item) ->
-        List.fold ~init:acc
-          ~f:(fun acc (f, _) ->
-            count_aux acc (CostDomain.BasicCost.decode (f v).Jsonbug_t.polynomial) )
-          CostKind.enabled_cost_kinds )
+        CostIssues.CostKindMap.fold
+          (fun _ CostIssues.{extract_cost_f} acc ->
+            count_aux acc (CostDomain.BasicCost.decode (extract_cost_f v).Jsonbug_t.polynomial) )
+          CostIssues.enabled_cost_map acc )
       costs
 
 
@@ -139,7 +139,8 @@ let to_map key_func report =
     ~init:String.Map.empty report
 
 
-let issue_of_cost ~kind cost_info ~delta ~prev_cost ~curr_cost =
+let issue_of_cost CostIssues.{complexity_increase_issue; zero_issue; infinite_issue} cost_info
+    ~delta ~prev_cost ~curr_cost =
   let file = cost_info.Jsonbug_t.loc.file in
   let method_name = cost_info.Jsonbug_t.procedure_name in
   let class_name =
@@ -152,11 +153,11 @@ let issue_of_cost ~kind cost_info ~delta ~prev_cost ~curr_cost =
   let procname = ExternalPerfData.make_void_signature_procname class_name method_name in
   let source_file = SourceFile.create ~warn_on_error:false file in
   let issue_type =
-    if CostDomain.BasicCost.is_top curr_cost then IssueType.infinite_cost_call ~kind
-    else if CostDomain.BasicCost.is_zero curr_cost then IssueType.zero_cost_call ~kind
+    if CostDomain.BasicCost.is_top curr_cost then infinite_issue
+    else if CostDomain.BasicCost.is_zero curr_cost then zero_issue
     else
       let is_on_cold_start = ExternalPerfData.in_profiler_data_map procname in
-      IssueType.complexity_increase ~kind ~is_on_cold_start
+      complexity_increase_issue ~is_on_cold_start
   in
   let curr_degree_with_term = CostDomain.BasicCost.get_degree_with_term curr_cost in
   let curr_cost_msg fmt () =
@@ -253,7 +254,7 @@ let issue_of_cost ~kind cost_info ~delta ~prev_cost ~curr_cost =
       DB < DA => introduced
  *)
 let of_costs ~(current_costs : Jsonbug_t.costs_report) ~(previous_costs : Jsonbug_t.costs_report) =
-  let fold_aux ~kind ~key:_ ~data (left, both, right) =
+  let fold_aux issue_spec ~key:_ ~data (left, both, right) =
     match data with
     | `Both (current, previous) ->
         let max_degree_polynomial l =
@@ -277,14 +278,14 @@ let of_costs ~(current_costs : Jsonbug_t.costs_report) ~(previous_costs : Jsonbu
           if cmp > 0 then
             (* introduced *)
             let left' =
-              issue_of_cost ~kind curr_cost_info ~delta:`Increased ~prev_cost ~curr_cost
+              issue_of_cost issue_spec curr_cost_info ~delta:`Increased ~prev_cost ~curr_cost
               |> concat_opt left
             in
             (left', both, right)
           else if cmp < 0 then
             (* fixed *)
             let right' =
-              issue_of_cost ~kind curr_cost_info ~delta:`Decreased ~prev_cost ~curr_cost
+              issue_of_cost issue_spec curr_cost_info ~delta:`Decreased ~prev_cost ~curr_cost
               |> concat_opt right
             in
             (left, both, right')
@@ -303,13 +304,13 @@ let of_costs ~(current_costs : Jsonbug_t.costs_report) ~(previous_costs : Jsonbu
   in
   let get_current_costs = decoded_costs current_costs in
   let get_previous_costs = decoded_costs previous_costs in
-  List.fold ~init:([], [], [])
-    ~f:(fun acc (extract_cost_f, kind) ->
+  CostIssues.CostKindMap.fold
+    (fun _kind CostIssues.({extract_cost_f} as issue_spec) acc ->
       Map.fold2
         (to_map (get_current_costs ~extract_cost_f))
         (to_map (get_previous_costs ~extract_cost_f))
-        ~f:(fold_aux ~kind) ~init:acc )
-    CostKind.enabled_cost_kinds
+        ~f:(fold_aux issue_spec) ~init:acc )
+    CostIssues.enabled_cost_map ([], [], [])
 
 
 (** Set operations should keep duplicated issues with identical hashes *)

infer/src/base/CostIssues.ml

@@ -0,0 +1,50 @@
+(*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *)
+open! IStd
+
+type issue_spec =
+  { extract_cost_f: Jsonbug_t.cost_item -> Jsonbug_t.cost_info
+  ; name: string
+  ; threshold: int option
+  ; complexity_increase_issue: is_on_cold_start:bool -> IssueType.t
+  ; expensive_issue: is_on_cold_start:bool -> IssueType.t
+  ; zero_issue: IssueType.t
+  ; infinite_issue: IssueType.t
+  ; top_and_bottom: bool }
+
+module CostKindMap = struct
+  include PrettyPrintable.MakePPMap (CostKind)
+
+  type no_value = |
+
+  let iter2 map1 map2 ~f =
+    let (_ : no_value t) =
+      merge
+        (fun k v1_opt v2_opt ->
+          (match (v1_opt, v2_opt) with Some v1, Some v2 -> f k v1 v2 | _ -> ()) ;
+          None )
+        map1 map2
+    in
+    ()
+end
+
+let enabled_cost_map =
+  List.fold CostKind.enabled_cost_kinds ~init:CostKindMap.empty
+    ~f:(fun acc CostKind.{kind; top_and_bottom} ->
+      let kind_spec =
+        { name= Format.asprintf "The %a" CostKind.pp kind
+        ; threshold= (if Config.use_cost_threshold then CostKind.to_threshold kind else None)
+        ; extract_cost_f= (fun c -> CostKind.to_json_cost_info c kind)
+        ; complexity_increase_issue=
+            (fun ~is_on_cold_start -> IssueType.complexity_increase ~kind ~is_on_cold_start)
+        ; expensive_issue=
+            (fun ~is_on_cold_start -> IssueType.expensive_cost_call ~kind ~is_on_cold_start)
+        ; zero_issue= IssueType.zero_cost_call ~kind
+        ; infinite_issue= IssueType.infinite_cost_call ~kind
+        ; top_and_bottom }
+      in
+      CostKindMap.add kind kind_spec acc )

infer/src/base/CostIssues.mli

@@ -0,0 +1,26 @@
+(*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *)
+
+open! IStd
+
+type issue_spec =
+  { extract_cost_f: Jsonbug_t.cost_item -> Jsonbug_t.cost_info
+  ; name: string
+  ; threshold: int option
+  ; complexity_increase_issue: is_on_cold_start:bool -> IssueType.t
+  ; expensive_issue: is_on_cold_start:bool -> IssueType.t
+  ; zero_issue: IssueType.t
+  ; infinite_issue: IssueType.t
+  ; top_and_bottom: bool }
+
+module CostKindMap : sig
+  include PrettyPrintable.PPMap with type key = CostKind.t
+
+  val iter2 : 'a t -> 'b t -> f:(key -> 'a -> 'b -> unit) -> unit
+end
+
+val enabled_cost_map : issue_spec CostKindMap.t

infer/src/base/IssueType.ml

@@ -85,6 +85,7 @@ end = struct
       issue
 
 
+  (** cost issues are already registered below.*)
   let from_cost_string ?(enabled = true) ?(is_on_cold_start = false) ~(kind : CostKind.t) s =
     let issue_type_base = Format.asprintf s (CostKind.to_issue_string kind) in
     let issue_type =
@@ -462,7 +463,7 @@ let zero_cost_call ~kind = from_cost_string ~enabled:false ~kind "ZERO_%s"
 
 (* register enabled cost issues *)
 let () =
-  List.iter CostKind.enabled_cost_kinds ~f:(fun (_, kind) ->
+  List.iter CostKind.enabled_cost_kinds ~f:(fun CostKind.{kind} ->
       List.iter [true; false] ~f:(fun is_on_cold_start ->
           let _ = zero_cost_call ~kind in
           let _ = expensive_cost_call ~kind ~is_on_cold_start in

infer/src/base/costKind.ml

@@ -22,15 +22,29 @@ let pp f k =
   let k_str =
     match k with
     | OperationCost ->
-        "OperationCost"
+        "Execution Cost"
     | AllocationCost ->
-        "AllocationCost"
+        "Allocation Cost"
     | IOCost ->
-        "IOCost"
+        "IO Cost"
   in
   F.pp_print_string f k_str
 
 
+let to_json_cost_info c = function
+  | OperationCost ->
+      c.Jsonbug_t.exec_cost
+  | AllocationCost ->
+      c.Jsonbug_t.alloc_cost
+  | IOCost ->
+      assert false
+
+
+(* We use this threshold to give error if the cost is above it.
+   Currently it's set randomly to 200 for OperationCost and 3 for AllocationCost. *)
+let to_threshold = function OperationCost -> Some 200 | AllocationCost -> Some 3 | IOCost -> None
+
+type kind_spec = {kind: t; (* for non-diff analysis *) top_and_bottom: bool}
+
 let enabled_cost_kinds =
-  [ ((fun c -> c.Jsonbug_t.exec_cost), OperationCost)
-  ; ((fun c -> c.Jsonbug_t.alloc_cost), AllocationCost) ]
+  [{kind= OperationCost; top_and_bottom= true}; {kind= AllocationCost; top_and_bottom= false}]

infer/src/base/costKind.mli

@@ -9,10 +9,16 @@ open! IStd
 
 type t = OperationCost | AllocationCost | IOCost [@@deriving compare]
 
+type kind_spec = {kind: t; (* for non-diff analysis *) top_and_bottom: bool}
+
 val compare : t -> t -> int
 
 val pp : Format.formatter -> t -> unit
 
 val to_issue_string : t -> string
 
-val enabled_cost_kinds : ((Jsonbug_t.cost_item -> Jsonbug_t.cost_info) * t) list
+val to_json_cost_info : Jsonbug_t.cost_item -> t -> Jsonbug_t.cost_info
+
+val enabled_cost_kinds : kind_spec list
+
+val to_threshold : t -> int option

infer/src/checkers/cost.ml

@@ -16,28 +16,6 @@ module Payload = SummaryPayload.Make (struct
   let field = Payloads.Fields.cost
 end)
 
-(* We use this threshold to give error if the cost is above it.
-   Currently it's set randomly to 200 for OperationCost and 3 for AllocationCost. *)
-module ReportConfig = struct
-  type t = {name: string; threshold: int option; top_and_bottom: bool}
-
-  let as_list =
-    [ ( CostKind.OperationCost
-      , { name= "The execution time"
-        ; threshold= Option.some_if Config.use_cost_threshold 200
-        ; top_and_bottom= true } )
-    ; ( CostKind.AllocationCost
-      , { name= "The allocations"
-        ; threshold= Option.some_if Config.use_cost_threshold 3
-        ; top_and_bottom= false } )
-    ; (CostKind.IOCost, {name= "The IOs"; threshold= None; top_and_bottom= false}) ]
-
-
-  let as_map =
-    List.fold as_list ~init:CostDomain.CostKindMap.empty ~f:(fun acc (k, v) ->
-        CostDomain.CostKindMap.add k v acc )
-end
-
 (* CFG modules used in several other modules  *)
 module InstrCFG = ProcCfg.NormalOneInstrPerNode
 module NodeCFG = ProcCfg.Normal
@@ -639,16 +617,19 @@ module ThresholdReports = struct
     | Threshold of BasicCost.t
     | ReportOn of {location: Location.t; cost: BasicCost.t}
 
-  type t = threshold_or_report CostDomain.CostKindMap.t
+  type t = threshold_or_report CostIssues.CostKindMap.t
 
-  let none = CostDomain.CostKindMap.empty
+  let none : t = CostIssues.CostKindMap.empty
 
   let config =
-    List.fold ReportConfig.as_list ~init:none ~f:(fun acc -> function
-      | k, ReportConfig.{threshold= Some threshold} ->
-          CostDomain.CostKindMap.add k (Threshold (BasicCost.of_int_exn threshold)) acc
-      | _ ->
-          acc )
+    CostIssues.CostKindMap.fold
+      (fun kind kind_spec acc ->
+        match kind_spec with
+        | CostIssues.{threshold= Some threshold} ->
+            CostIssues.CostKindMap.add kind (Threshold (BasicCost.of_int_exn threshold)) acc
+        | _ ->
+            acc )
+      CostIssues.enabled_cost_map none
 end
 
 (*
@@ -676,7 +657,7 @@ module WorstCaseCost = struct
     in
     let costs = CostDomain.plus costs node_cost in
     let reports =
-      CostDomain.CostKindMap.merge
+      CostIssues.CostKindMap.merge
         (fun _kind threshold_or_report_opt cost_opt ->
           match (threshold_or_report_opt, cost_opt) with
           | None, _ ->
@@ -713,7 +694,8 @@ module WorstCaseCost = struct
 end
 
 module Check = struct
-  let report_threshold proc_desc summary ~name ~location ~cost ~threshold ~kind =
+  let report_threshold proc_desc summary ~name ~location ~cost CostIssues.{expensive_issue}
+      ~threshold =
     let report_issue_type =
       L.(debug Analysis Medium)
         "@\n\n++++++ Checking error type for %a **** @\n" Typ.Procname.pp
@@ -721,7 +703,7 @@ module Check = struct
       let is_on_cold_start =
         ExternalPerfData.in_profiler_data_map (Procdesc.get_proc_name proc_desc)
       in
-      IssueType.expensive_cost_call ~kind ~is_on_cold_start
+      expensive_issue ~is_on_cold_start
     in
     let degree_str = BasicCost.degree_str cost in
     let message =
@@ -739,7 +721,7 @@ module Check = struct
       ~extras:(compute_errlog_extras cost) report_issue_type message
 
 
-  let report_top_and_bottom kind proc_desc summary ~name ~cost =
+  let report_top_and_bottom proc_desc summary ~name ~cost CostIssues.{zero_issue; infinite_issue} =
     let report issue suffix =
       let message =
         F.asprintf "%s of the function %a %s" name Typ.Procname.pp
@@ -751,23 +733,23 @@ module Check = struct
         ~ltr:(BasicCost.polynomial_traces cost)
         ~extras:(compute_errlog_extras cost) summary issue message
     in
-    if BasicCost.is_top cost then report (IssueType.infinite_cost_call ~kind) "cannot be computed"
-    else if BasicCost.is_zero cost then report (IssueType.zero_cost_call ~kind) "is zero"
+    if BasicCost.is_top cost then report infinite_issue "cannot be computed"
+    else if BasicCost.is_zero cost then report zero_issue "is zero"
 
 
   let check_and_report WorstCaseCost.{costs; reports} proc_desc summary =
     let pname = Procdesc.get_proc_name proc_desc in
     if not (Typ.Procname.is_java_access_method pname) then (
-      CostDomain.CostKindMap.iter2 ReportConfig.as_map reports
-        ~f:(fun kind ReportConfig.{name; threshold} -> function
+      CostIssues.CostKindMap.iter2 CostIssues.enabled_cost_map reports
+        ~f:(fun _kind (CostIssues.{name; threshold} as kind_spec) -> function
         | ThresholdReports.Threshold _ ->
             ()
         | ThresholdReports.ReportOn {location; cost} ->
-            report_threshold proc_desc summary ~name ~location ~cost
-              ~threshold:(Option.value_exn threshold) ~kind ) ;
-      CostDomain.CostKindMap.iter2 ReportConfig.as_map costs
-        ~f:(fun kind ReportConfig.{name; top_and_bottom} cost ->
-          if top_and_bottom then report_top_and_bottom kind proc_desc summary ~name ~cost ) )
+            report_threshold proc_desc summary ~name ~location ~cost kind_spec
+              ~threshold:(Option.value_exn threshold) ) ;
+      CostIssues.CostKindMap.iter2 CostIssues.enabled_cost_map costs
+        ~f:(fun _kind (CostIssues.{name; top_and_bottom} as issue_spec) cost ->
+          if top_and_bottom then report_top_and_bottom proc_desc summary ~name ~cost issue_spec ) )
 end
 
 type bound_map = BasicCost.t Node.IdMap.t

infer/src/checkers/costDomain.ml

@@ -9,29 +9,13 @@ open! IStd
 module F = Format
 module BasicCost = Polynomials.NonNegativePolynomial
 
-module CostKindMap = struct
-  include PrettyPrintable.MakePPMap (CostKind)
-
-  type no_value = |
-
-  let iter2 map1 map2 ~f =
-    let (_ : no_value t) =
-      merge
-        (fun k v1_opt v2_opt ->
-          (match (v1_opt, v2_opt) with Some v1, Some v2 -> f k v1 v2 | _ -> ()) ;
-          None )
-        map1 map2
-    in
-    ()
-end
-
 (**
   Module to simulate a record 
     {OperationCost:BasicCost.t; AllocationCost: BasicCost.t; IOCost:BasicCost.t} with a map
     {OperationCost, AllocationCost, IOCost} -> BasicCost.t
 *)
 module VariantCostMap = struct
-  include PrettyPrintable.PPMonoMapOfPPMap (CostKindMap) (BasicCost)
+  include PrettyPrintable.PPMonoMapOfPPMap (CostIssues.CostKindMap) (BasicCost)
 
   let[@warning "-32"] add _ = Logging.die InternalError "Don't call me"