From 898dd104c8b153f333977ded5aae695478717e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Mon, 17 Jun 2019 08:39:25 -0700 Subject: [PATCH] [cost] Invoke Cost issues only once Reviewed By: mbouaziz Differential Revision: D15853454 fbshipit-source-id: 41ec36392 --- infer/src/backend/Differential.ml | 31 ++++++++------- infer/src/base/CostIssues.ml | 50 +++++++++++++++++++++++ infer/src/base/CostIssues.mli | 26 ++++++++++++ infer/src/base/IssueType.ml | 3 +- infer/src/base/costKind.ml | 24 ++++++++--- infer/src/base/costKind.mli | 8 +++- infer/src/checkers/cost.ml | 66 +++++++++++-------------------- infer/src/checkers/costDomain.ml | 18 +-------- 8 files changed, 145 insertions(+), 81 deletions(-) create mode 100644 infer/src/base/CostIssues.ml create mode 100644 infer/src/base/CostIssues.mli diff --git a/infer/src/backend/Differential.ml b/infer/src/backend/Differential.ml index 90406d3a6..8c76957fc 100644 --- a/infer/src/backend/Differential.ml +++ b/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 *) diff --git a/infer/src/base/CostIssues.ml b/infer/src/base/CostIssues.ml new file mode 100644 index 000000000..543fab403 --- /dev/null +++ b/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 ) diff --git a/infer/src/base/CostIssues.mli b/infer/src/base/CostIssues.mli new file mode 100644 index 000000000..83ddfcc64 --- /dev/null +++ b/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 diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index be2106f68..3d9dd3729 100644 --- a/infer/src/base/IssueType.ml +++ b/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 diff --git a/infer/src/base/costKind.ml b/infer/src/base/costKind.ml index ff4f9afbf..8748fae2f 100644 --- a/infer/src/base/costKind.ml +++ b/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}] diff --git a/infer/src/base/costKind.mli b/infer/src/base/costKind.mli index 754382f5b..fb40aa941 100644 --- a/infer/src/base/costKind.mli +++ b/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 diff --git a/infer/src/checkers/cost.ml b/infer/src/checkers/cost.ml index 9e551bee3..7927ddd80 100644 --- a/infer/src/checkers/cost.ml +++ b/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 diff --git a/infer/src/checkers/costDomain.ml b/infer/src/checkers/costDomain.ml index 0320ce56f..ed4df5bb4 100644 --- a/infer/src/checkers/costDomain.ml +++ b/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"