From c078bf5a12c919501d98b0a5a38ff0b64d0197e4 Mon Sep 17 00:00:00 2001 From: Martino Luca Date: Fri, 14 Sep 2018 03:06:24 -0700 Subject: [PATCH] [Perf][CI] Differential of costs, based on degree variation Summary: First version of differential for costs, based on polynomial's degree's variation. The rule is very simple: For a given polynomial that is available before and after a diff, `if degree_before > degree_after`, then the issue becomes `fixed`. Instead, `if degree_before < degree_after`, then the issue becomes `introduced`. Reviewed By: ezgicicek Differential Revision: D9810150 fbshipit-source-id: d08285926 --- infer/src/backend/Differential.ml | 115 ++++++++++++++++-- infer/src/backend/InferPrint.ml | 6 +- infer/src/backend/InferPrint.mli | 2 + infer/src/base/IssueType.ml | 2 + infer/src/base/IssueType.mli | 2 + infer/src/bufferoverrun/itv.ml | 20 +++ infer/src/bufferoverrun/itv.mli | 4 + .../costs_summary.json.exp | 2 +- .../differential_of_costs_report/fixed.exp | 1 + .../introduced.exp | 2 + .../src/DiffExample.java.current | 16 ++- .../src/DiffExample.java.previous | 19 ++- 12 files changed, 170 insertions(+), 21 deletions(-) diff --git a/infer/src/backend/Differential.ml b/infer/src/backend/Differential.ml index d382831d5..cda4e59d9 100644 --- a/infer/src/backend/Differential.ml +++ b/infer/src/backend/Differential.ml @@ -122,15 +122,110 @@ type t = ; preexisting: Jsonbug_t.report ; costs_summary: Yojson.Basic.json } +let to_map key_func report = + List.fold_left + ~f:(fun map elt -> Map.add_multi map ~key:(key_func elt) ~data:elt) + ~init:String.Map.empty report + + +let issue_of_cost (cost_info, cost_polynomial) ~delta ~prev_cost ~curr_cost = + let source_file = SourceFile.create ~warn_on_error:false cost_info.Jsonbug_t.loc.file in + let issue_type = + if CostDomain.BasicCost.is_top cost_polynomial then IssueType.infinite_execution_time_call + else if CostDomain.BasicCost.is_zero cost_polynomial then IssueType.zero_execution_time_call + else IssueType.performance_variation + in + let qualifier = + let pp_delta fmt delta = + match delta with + | `Decreased -> + Format.fprintf fmt "decreased" + | `Increased -> + Format.fprintf fmt "increased" + in + Format.asprintf "Max degree %a from %a to %a. Cost is %a (degree is %a)" pp_delta delta + CostDomain.BasicCost.pp_degree prev_cost CostDomain.BasicCost.pp_degree curr_cost + CostDomain.BasicCost.pp cost_polynomial CostDomain.BasicCost.pp_degree cost_polynomial + in + { Jsonbug_j.bug_type= issue_type.IssueType.unique_id + ; qualifier + ; severity= Exceptions.severity_string Exceptions.Warning + ; visibility= Exceptions.string_of_visibility Exceptions.Exn_user + ; line= cost_info.Jsonbug_t.loc.lnum + ; column= cost_info.Jsonbug_t.loc.cnum + ; procedure= cost_info.Jsonbug_t.procedure_id + ; procedure_start_line= 0 + ; file= cost_info.Jsonbug_t.loc.file + ; bug_trace= [] + ; key= "" + ; node_key= None + ; hash= cost_info.Jsonbug_t.hash + ; dotty= None + ; infer_source_loc= None + ; bug_type_hum= issue_type.IssueType.hum + ; linters_def_file= None + ; doc_url= None + ; traceview_id= None + ; censored_reason= InferPrint.censored_reason issue_type source_file + ; access= None + ; extras= None } + + +(** Differential of cost reports, based on degree variations. + Compare degree_before (DB), and degree_after (DA): + DB > DA => fixed + DB < DA => introduced + *) +let of_costs ~(current_costs : Jsonbug_t.costs_report) ~(previous_costs : Jsonbug_t.costs_report) = + let fold_aux ~key:_ ~data (left, both, right) = + match data with + | `Both (current, previous) -> + let max_degree_polynomial l = + let max = + List.max_elt l ~compare:(fun (_, c1) (_, c2) -> + CostDomain.BasicCost.compare_by_degree c1 c2 ) + in + Option.value_exn max |> snd + in + let curr_cost = max_degree_polynomial current in + let prev_cost = max_degree_polynomial previous in + let cmp = CostDomain.BasicCost.compare_by_degree curr_cost prev_cost in + if cmp > 0 then + (* introduced *) + let left' = + List.rev_map_append + ~f:(fun c -> issue_of_cost c ~delta:`Increased ~prev_cost ~curr_cost) + current left + in + (left', both, right) + else if cmp < 0 then + (* fixed *) + let right' = + List.rev_map_append + ~f:(fun c -> issue_of_cost c ~delta:`Decreased ~prev_cost ~curr_cost) + current right + in + (left, both, right') + else + (* preexisting costs are not issues, since their values have not changed *) + (left, both, right) + | `Left _ | `Right _ -> + (* costs available only on one of the two reports are discarded, since no comparison can be made *) + (left, both, right) + in + let key_func (cost_info, _) = cost_info.Jsonbug_t.hash in + let to_map = to_map key_func in + let decoded_costs costs = + List.map costs ~f:(fun c -> (c, CostDomain.BasicCost.decode c.Jsonbug_t.polynomial)) + in + let current_costs' = decoded_costs current_costs in + let previous_costs' = decoded_costs previous_costs in + Map.fold2 (to_map current_costs') (to_map previous_costs') ~f:fold_aux ~init:([], [], []) + + (** Set operations should keep duplicated issues with identical hashes *) let of_reports ~(current_report : Jsonbug_t.report) ~(previous_report : Jsonbug_t.report) ~(current_costs : Jsonbug_t.costs_report) ~(previous_costs : Jsonbug_t.costs_report) : t = - let to_map report = - List.fold_left - ~f:(fun map (issue : Jsonbug_t.jsonbug) -> - Map.add_multi map ~key:issue.Jsonbug_t.hash ~data:issue ) - ~init:String.Map.empty report - in let fold_aux ~key:_ ~data (left, both, right) = match data with | `Left left' -> @@ -141,10 +236,16 @@ let of_reports ~(current_report : Jsonbug_t.report) ~(previous_report : Jsonbug_ (left, both, List.rev_append right' right) in let introduced, preexisting, fixed = + let key_func (issue : Jsonbug_t.jsonbug) = issue.hash in + let to_map = to_map key_func in Map.fold2 (to_map current_report) (to_map previous_report) ~f:fold_aux ~init:([], [], []) in let costs_summary = CostsSummary.to_json ~current_costs ~previous_costs in - {introduced= dedup introduced; fixed= dedup fixed; preexisting= dedup preexisting; costs_summary} + let introduced_costs, preexisting_costs, fixed_costs = of_costs ~current_costs ~previous_costs in + { introduced= List.rev_append introduced_costs (dedup introduced) + ; fixed= List.rev_append fixed_costs (dedup fixed) + ; preexisting= List.rev_append preexisting_costs (dedup preexisting) + ; costs_summary } let to_files {introduced; fixed; preexisting; costs_summary} destdir = diff --git a/infer/src/backend/InferPrint.ml b/infer/src/backend/InferPrint.ml index aea7a2855..5e8a0439d 100644 --- a/infer/src/backend/InferPrint.ml +++ b/infer/src/backend/InferPrint.ml @@ -347,13 +347,11 @@ module JsonCostsPrinter = MakeJsonListPrinter (struct if Config.developer_mode then Some { Jsonbug_t.hum_polynomial= Format.asprintf "%a" CostDomain.BasicCost.pp post - ; hum_degree= - Option.value_map ~default:"Top" ~f:string_of_int - (CostDomain.BasicCost.degree post) } + ; hum_degree= Format.asprintf "%a" CostDomain.BasicCost.pp_degree post } else None in let cost_item = - let file = SourceFile.to_string loc.Location.file in + let file = SourceFile.to_rel_path loc.Location.file in { Jsonbug_t.hash= compute_hash ~severity:"" ~bug_type:"" ~proc_name ~file ~qualifier:"" ; loc= {file; lnum= loc.Location.line; cnum= loc.Location.col; enum= -1} ; procedure_id= Typ.Procname.to_string proc_name diff --git a/infer/src/backend/InferPrint.mli b/infer/src/backend/InferPrint.mli index bd3233f01..bf4988752 100644 --- a/infer/src/backend/InferPrint.mli +++ b/infer/src/backend/InferPrint.mli @@ -7,4 +7,6 @@ open! IStd +val censored_reason : IssueType.t -> SourceFile.t -> string + val main : report_json:string option -> unit diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 6c0170524..08c12481a 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -311,6 +311,8 @@ let nullable_dereference = from_string "NULLABLE_DEREFERENCE" let parameter_not_null_checked = from_string "PARAMETER_NOT_NULL_CHECKED" +let performance_variation = from_string "PERFORMANCE_VARIATION" + let pointer_size_mismatch = from_string "POINTER_SIZE_MISMATCH" let precondition_not_found = from_string "PRECONDITION_NOT_FOUND" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index a22a39851..4cd5e66fb 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -217,6 +217,8 @@ val nullable_dereference : t val parameter_not_null_checked : t +val performance_variation : t + val pointer_size_mismatch : t val precondition_not_found : t diff --git a/infer/src/bufferoverrun/itv.ml b/infer/src/bufferoverrun/itv.ml index f0e68ef2e..2692c955f 100644 --- a/infer/src/bufferoverrun/itv.ml +++ b/infer/src/bufferoverrun/itv.ml @@ -396,6 +396,26 @@ module NonNegativePolynomial = struct match p with Top -> None | NonTop p -> Some (NonNegativeNonTopPolynomial.degree p) + let compare_by_degree p1 p2 = + match (p1, p2) with + | Top, Top -> + 0 + | Top, NonTop _ -> + 1 + | NonTop _, Top -> + -1 + | NonTop p1, NonTop p2 -> + NonNegativeNonTopPolynomial.degree p1 - NonNegativeNonTopPolynomial.degree p2 + + + let pp_degree fmt p = + match p with + | Top -> + Format.pp_print_string fmt "Top" + | NonTop p -> + Format.pp_print_int fmt (NonNegativeNonTopPolynomial.degree p) + + let encode astate = Marshal.to_string astate [] |> B64.encode let decode enc_str = Marshal.from_string (B64.decode enc_str) 0 diff --git a/infer/src/bufferoverrun/itv.mli b/infer/src/bufferoverrun/itv.mli index 3a3774d12..f55ef7bb5 100644 --- a/infer/src/bufferoverrun/itv.mli +++ b/infer/src/bufferoverrun/itv.mli @@ -54,6 +54,10 @@ module NonNegativePolynomial : sig val degree : astate -> int option + val compare_by_degree : astate -> astate -> int + + val pp_degree : Format.formatter -> astate -> unit + val encode : astate -> string val decode : string -> astate diff --git a/infer/tests/build_systems/differential_of_costs_report/costs_summary.json.exp b/infer/tests/build_systems/differential_of_costs_report/costs_summary.json.exp index ff00d3fcb..574550d2e 100644 --- a/infer/tests/build_systems/differential_of_costs_report/costs_summary.json.exp +++ b/infer/tests/build_systems/differential_of_costs_report/costs_summary.json.exp @@ -1 +1 @@ -{"top":{"current":1,"previous":0},"zero":{"current":1,"previous":0},"degrees":[{"degree":0,"current":2,"previous":1},{"degree":1,"current":1,"previous":1},{"degree":2,"current":0,"previous":1}]} \ No newline at end of file +{"top":{"current":1,"previous":0},"zero":{"current":1,"previous":0},"degrees":[{"degree":0,"current":2,"previous":2},{"degree":1,"current":1,"previous":1},{"degree":2,"current":0,"previous":1}]} \ No newline at end of file diff --git a/infer/tests/build_systems/differential_of_costs_report/fixed.exp b/infer/tests/build_systems/differential_of_costs_report/fixed.exp index e69de29bb..36f1b8734 100644 --- a/infer/tests/build_systems/differential_of_costs_report/fixed.exp +++ b/infer/tests/build_systems/differential_of_costs_report/fixed.exp @@ -0,0 +1 @@ +ZERO_EXECUTION_TIME_CALL, no_bucket, src/DiffExample.java, void DiffExample.f2(int), 27 diff --git a/infer/tests/build_systems/differential_of_costs_report/introduced.exp b/infer/tests/build_systems/differential_of_costs_report/introduced.exp index e69de29bb..ee65a9ec8 100644 --- a/infer/tests/build_systems/differential_of_costs_report/introduced.exp +++ b/infer/tests/build_systems/differential_of_costs_report/introduced.exp @@ -0,0 +1,2 @@ +PERFORMANCE_VARIATION, no_bucket, src/DiffExample.java, int DiffExample.f4(int), 39 +INFINITE_EXECUTION_TIME_CALL, no_bucket, src/DiffExample.java, void DiffExample.f1(int), 19 diff --git a/infer/tests/build_systems/differential_of_costs_report/src/DiffExample.java.current b/infer/tests/build_systems/differential_of_costs_report/src/DiffExample.java.current index 7bb845896..a804992e8 100644 --- a/infer/tests/build_systems/differential_of_costs_report/src/DiffExample.java.current +++ b/infer/tests/build_systems/differential_of_costs_report/src/DiffExample.java.current @@ -6,11 +6,17 @@ */ // This class has the following costs: -// 1 top, 1 bottom, 2 constant (incl. constructor), 1 linear +// 1 bottom (zero), 2 constant, 1 linear, 1 top +// constructor: constant +// f1: top +// f2: bottom (zero) +// f3: constant +// f4: linear + public class DiffExample { // cost: top - private static void top() { + private static void f1(int k) { int i = 0; while (i >=0) { i++; @@ -18,10 +24,10 @@ public class DiffExample { } // cost: bottom (0) - private static void bottom() {} + private static void f2(int k) {} // cost: constant (5) - private static int constant() { + private static int f3() { int i, j; i = 17; j = 31; @@ -30,7 +36,7 @@ public class DiffExample { } // cost: linear - private static int linear(int k) { + private static int f4(int k) { for (int i = 0; i < k; i++) { } return 0; diff --git a/infer/tests/build_systems/differential_of_costs_report/src/DiffExample.java.previous b/infer/tests/build_systems/differential_of_costs_report/src/DiffExample.java.previous index 8326e60aa..598c40ce7 100644 --- a/infer/tests/build_systems/differential_of_costs_report/src/DiffExample.java.previous +++ b/infer/tests/build_systems/differential_of_costs_report/src/DiffExample.java.previous @@ -6,19 +6,30 @@ */ // This class has the following costs: -// 1 constant (constructor), 1 linear, 1 quadratic +// 2 constant, 1 linear, 1 quadratic +// constructor: constant +// f1: linear +// f2: quadratic +// f4: constant + public class DiffExample { // cost: linear - private static int linear(int k) { + private static int f1(int k) { for (int i = 0; i < k; i++) { } return 0; } // cost: quadratic - private static void quadratic(int k) { + private static void f2(int k) { for (int i = 0; i < k; i++) { - linear(k); + f1(k); } } + + // cost: constant + private static int f4(int k) { + int i = 1; + return i + k; + } }