[Perf] Perform comparisons of costs-reports by using costs-report.json files

Reviewed By: ezgicicek, mbouaziz

Differential Revision: D9634617

fbshipit-source-id: 6d349ce40

Makefile

@@ -77,6 +77,7 @@ endif # BUILD_C_ANALYZERS
 ifeq ($(BUILD_JAVA_ANALYZERS),yes)
 BUILD_SYSTEMS_TESTS += \
 	differential_interesting_paths_filter \
+  differential_of_costs_report \
   differential_skip_anonymous_class_renamings \
   differential_skip_duplicated_types_on_filenames \
   differential_skip_duplicated_types_on_filenames_with_renamings \

infer/src/backend/Differential.ml

@@ -53,40 +53,30 @@ let dedup (issues : Jsonbug_t.jsonbug list) =
   |> snd |> sort_by_location
 
 
-module CostsSummary : sig
-  val to_json :
-    current_report:Jsonbug_t.report -> previous_report:Jsonbug_t.report -> Yojson.Basic.json
-end = struct
+module CostsSummary = struct
   type 'a count = {top: 'a; zero: 'a; degrees: 'a Int.Map.t}
 
   let init = {top= 0; zero= 0; degrees= Int.Map.empty}
 
   type previous_current = {previous: int; current: int}
 
-  let zero_token_str =
-    Format.asprintf "%a" Itv.NonNegativePolynomial.pp Itv.NonNegativePolynomial.zero
-
-
-  let top_token_str =
-    Format.asprintf "%a" Itv.NonNegativePolynomial.pp Itv.NonNegativePolynomial.top
-
-
-  let count report =
-    let count_aux t (e : Jsonbug_t.extra) =
-      match e with
-      | {cost_polynomial= Some cp} when String.equal cp zero_token_str ->
-          {t with zero= t.zero + 1}
-      | {cost_polynomial= Some cp; cost_degree= None} when String.equal cp top_token_str ->
+  let count costs =
+    let count_aux t (e : Itv.NonNegativePolynomial.astate) =
+      match Itv.NonNegativePolynomial.degree e with
+      | None ->
+          assert (Itv.NonNegativePolynomial.is_top e) ;
           {t with top= t.top + 1}
-      | {cost_degree= Some v} ->
-          let degrees = Int.Map.update t.degrees v ~f:(function None -> 1 | Some x -> x + 1) in
+      | Some d when Itv.NonNegativePolynomial.is_zero e ->
+          assert (Int.equal d 0) ;
+          {t with zero= t.zero + 1}
+      | Some d ->
+          let degrees = Int.Map.update t.degrees d ~f:(function None -> 1 | Some x -> x + 1) in
           {t with degrees}
-      | {cost_degree= None} ->
-          t
     in
     List.fold ~init
-      ~f:(fun acc v -> match v.Jsonbug_t.extras with None -> acc | Some v -> count_aux acc v)
-      report
+      ~f:(fun acc (v : Jsonbug_t.cost_item) ->
+        count_aux acc (Itv.NonNegativePolynomial.decode v.polynomial) )
+      costs
 
 
   let pair_counts ~current_counts ~previous_counts =
@@ -107,9 +97,9 @@ end = struct
     ; degrees= compute_degrees current_counts.degrees previous_counts.degrees }
 
 
-  let to_json ~current_report ~previous_report =
-    let current_counts = count current_report in
-    let previous_counts = count previous_report in
+  let to_json ~current_costs ~previous_costs =
+    let current_counts = count current_costs in
+    let previous_counts = count previous_costs in
     let paired_counts = pair_counts ~current_counts ~previous_counts in
     let json_degrees =
       Int.Map.to_alist ~key_order:`Increasing paired_counts.degrees
@@ -133,7 +123,8 @@ type t =
   ; costs_summary: Yojson.Basic.json }
 
 (** Set operations should keep duplicated issues with identical hashes *)
-let of_reports ~(current_report : Jsonbug_t.report) ~(previous_report : Jsonbug_t.report) : t =
+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 -> Map.add_multi map ~key:issue.Jsonbug_t.hash ~data:issue)
@@ -151,7 +142,7 @@ let of_reports ~(current_report : Jsonbug_t.report) ~(previous_report : Jsonbug_
   let introduced, preexisting, fixed =
     Map.fold2 (to_map current_report) (to_map previous_report) ~f:fold_aux ~init:([], [], [])
   in
-  let costs_summary = CostsSummary.to_json ~current_report ~previous_report in
+  let costs_summary = CostsSummary.to_json ~current_costs ~previous_costs in
   {introduced= dedup introduced; fixed= dedup fixed; preexisting= dedup preexisting; costs_summary}
 
 

infer/src/backend/Differential.mli

@@ -13,6 +13,11 @@ type t =
   ; preexisting: Jsonbug_t.report
   ; costs_summary: Yojson.Basic.json }
 
-val of_reports : current_report:Jsonbug_t.report -> previous_report:Jsonbug_t.report -> t
+val 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
 
 val to_files : t -> string -> unit

infer/src/base/Config.ml

@@ -973,6 +973,18 @@ and cost_invariant_by_default =
     "[Cost]Consider functions to be invariant by default"
 
 
+and costs_current =
+  CLOpt.mk_path_opt ~long:"costs-current"
+    ~in_help:InferCommand.[(ReportDiff, manual_generic)]
+    "Costs report of the latest revision"
+
+
+and costs_previous =
+  CLOpt.mk_path_opt ~long:"costs-previous"
+    ~in_help:InferCommand.[(ReportDiff, manual_generic)]
+    "Costs report of the base revision to use for comparison"
+
+
 and current_to_previous_script =
   CLOpt.mk_string_opt ~long:"current-to-previous-script"
     ~in_help:InferCommand.[(Diff, manual_generic)]
@@ -2548,6 +2560,10 @@ and cost = !cost
 
 and cost_invariant_by_default = !cost_invariant_by_default
 
+and costs_current = !costs_current
+
+and costs_previous = !costs_previous
+
 and current_to_previous_script = !current_to_previous_script
 
 and crashcontext = !crashcontext

infer/src/base/Config.mli

@@ -291,6 +291,10 @@ val cost : bool
 
 val cost_invariant_by_default : bool
 
+val costs_current : string option
+
+val costs_previous : string option
+
 val crashcontext : bool
 
 val current_to_previous_script : string option

infer/src/bufferoverrun/itv.ml

@@ -397,6 +397,8 @@ module NonNegativePolynomial = struct
 
 
   let encode astate = Marshal.to_string astate [] |> B64.encode
+
+  let decode enc_str = Marshal.from_string (B64.decode enc_str) 0
 end
 
 module ItvRange = struct

infer/src/bufferoverrun/itv.mli

@@ -55,6 +55,8 @@ module NonNegativePolynomial : sig
   val degree : astate -> int option
 
   val encode : astate -> string
+
+  val decode : string -> astate
 end
 
 module ItvRange : sig

infer/src/infer.ml

@@ -133,14 +133,16 @@ let () =
         InferPrint.main ~report_json:None
     | ReportDiff ->
         (* at least one report must be passed in input to compute differential *)
-        ( match (Config.report_current, Config.report_previous) with
-        | None, None ->
+        ( match Config.(report_current, report_previous, costs_current, costs_previous) with
+        | None, None, None, None ->
             L.(die UserError)
-              "Expected at least one argument among '--report-current' and '--report-previous'"
+              "Expected at least one argument among '--report-current', '--report-previous', \
+               '--costs-current', and '--costs-previous'"
         | _ ->
             () ) ;
         ReportDiff.reportdiff ~current_report:Config.report_current
-          ~previous_report:Config.report_previous
+          ~previous_report:Config.report_previous ~current_costs:Config.costs_current
+          ~previous_costs:Config.costs_previous
     | Diff ->
         Diff.diff (Lazy.force Driver.mode_from_command_line)
     | Explore when Config.procedures ->

infer/src/integration/Diff.ml

@@ -40,8 +40,14 @@ let checkout revision =
 let save_report revision =
   let report_name = Config.results_dir ^/ F.asprintf "report-%a.json" pp_revision revision in
   Unix.rename ~src:Config.(results_dir ^/ report_json) ~dst:report_name ;
+  let costs_report_name =
+    Config.results_dir ^/ F.asprintf "costs-report-%a.json" pp_revision revision
+  in
+  Unix.rename ~src:Config.(results_dir ^/ costs_report_json) ~dst:costs_report_name ;
   L.progress "Results for the %a revision stored in %s@\n" pp_revision revision report_name ;
-  report_name
+  L.progress "Costs data for the %a revision stored in %s@\n" pp_revision revision
+    costs_report_name ;
+  (report_name, costs_report_name)
 
 
 let gen_previous_driver_mode script =
@@ -64,7 +70,7 @@ let diff driver_mode =
   let changed_files = Driver.read_config_changed_files () in
   Driver.capture driver_mode ~changed_files ;
   Driver.analyze_and_report ~suppress_console_report:true driver_mode ~changed_files ;
-  let current_report = Some (save_report Current) in
+  let current_report, current_costs = save_report Current in
   (* Some files in the current checkout may be deleted in the old checkout. If we kept the results of the previous capture and analysis around, we would report issues on these files again in the previous checkout, which is wrong. Do not do anything too smart for now and just delete all results from the analysis of the current checkout. *)
   ResultsDir.delete_capture_and_analysis_data () ;
   (* TODO(t15553258) bail if nothing to analyze (configurable, some people might care about bugs
@@ -77,8 +83,10 @@ let diff driver_mode =
   Driver.capture previous_driver_mode ~changed_files ;
   Driver.analyze_and_report ~suppress_console_report:true previous_driver_mode ~changed_files ;
   checkout Current ;
-  let previous_report = Some (save_report Previous) in
+  let previous_report, previous_costs = save_report Previous in
   (* compute differential *)
-  ReportDiff.reportdiff ~current_report ~previous_report ;
+  ReportDiff.reportdiff ~current_report:(Some current_report)
+    ~previous_report:(Some previous_report) ~current_costs:(Some current_costs)
+    ~previous_costs:(Some previous_costs) ;
   Driver.run_epilogue driver_mode ;
   ()

infer/src/integration/ReportDiff.ml

@@ -6,17 +6,21 @@
  *)
 open! IStd
 
-let reportdiff ~current_report:current_report_fname ~previous_report:previous_report_fname =
-  let load_report filename_opt : Jsonbug_t.report =
-    let empty_report = [] in
-    Option.value_map
-      ~f:(fun filename -> Jsonbug_j.report_of_string (In_channel.read_all filename))
-      ~default:empty_report filename_opt
+let reportdiff ~current_report:current_report_fname ~previous_report:previous_report_fname
+    ~current_costs:current_costs_fname ~previous_costs:previous_costs_fname =
+  let load_aux ~f filename_opt =
+    Option.value_map ~f:(fun filename -> f (In_channel.read_all filename)) ~default:[] filename_opt
   in
+  let load_report = load_aux ~f:Jsonbug_j.report_of_string in
+  let load_costs = load_aux ~f:Jsonbug_j.costs_report_of_string in
   let current_report = load_report current_report_fname in
   let previous_report = load_report previous_report_fname in
+  let current_costs = load_costs current_costs_fname in
+  let previous_costs = load_costs previous_costs_fname in
   let diff =
-    let unfiltered_diff = Differential.of_reports ~current_report ~previous_report in
+    let unfiltered_diff =
+      Differential.of_reports ~current_report ~previous_report ~current_costs ~previous_costs
+    in
     if Config.filtering then
       let file_renamings =
         match Config.file_renamings with

infer/src/integration/ReportDiff.mli

@@ -7,4 +7,9 @@
 
 open! IStd
 
-val reportdiff : current_report:string option -> previous_report:string option -> unit
+val reportdiff :
+     current_report:string option
+  -> previous_report:string option
+  -> current_costs:string option
+  -> previous_costs:string option
+  -> unit

infer/src/unit/DifferentialFiltersTests.ml

@@ -167,7 +167,11 @@ let test_skip_duplicated_types_on_filenames =
         [ {current= "file_2'.java"; previous= "file_2.java"}
         ; {current= "file_1'.java"; previous= "file_1.java"} ])
   in
-  let diff = Differential.of_reports ~current_report ~previous_report in
+  let current_costs = [] in
+  let previous_costs = [] in
+  let diff =
+    Differential.of_reports ~current_report ~previous_report ~current_costs ~previous_costs
+  in
   let diff' =
     DifferentialFilters.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.skip_duplicated_types_on_filenames
       renamings diff

infer/src/unit/DifferentialTests.ml

@@ -23,7 +23,11 @@ let previous_report =
   ; create_fake_jsonbug ~hash:"1" () ]
 
 
-let diff = Differential.of_reports ~current_report ~previous_report
+let current_costs = []
+
+let previous_costs = []
+
+let diff = Differential.of_reports ~current_report ~previous_report ~current_costs ~previous_costs
 
 (* Sets operations should keep duplicated issues with identical hashes *)
 let test_diff_keeps_duplicated_hashes =

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

@@ -0,0 +1 @@
+{"top":{"current":0,"previous":0},"zero":{"current":0,"previous":0},"degrees":[]}

infer/tests/build_systems/differential_of_costs_report/Makefile

@@ -0,0 +1,24 @@
+# Copyright (c) 2017-present, Facebook, Inc.
+#
+# This source code is licensed under the MIT license found in the
+# LICENSE file in the root directory of this source tree.
+
+# E2E test for anonymous class renaming
+
+TESTS_DIR = ../..
+SOURCES = src/DiffExample.java.current src/DiffExample.java.previous
+CLEAN_EXTRA = src/Diff*.java *.class
+
+include $(TESTS_DIR)/differential.make
+
+$(CURRENT_REPORT) $(PREVIOUS_REPORT): $(JAVA_DEPS)
+
+$(CURRENT_REPORT):
+	$(QUIET)$(COPY) src/DiffExample.java.current src/DiffExample.java
+	$(QUIET)$(call silent_on_success,Testing Differential skips anon class renamings: current,\
+	$(INFER_BIN) --cost-only -o $(CURRENT_DIR) -- $(JAVAC) src/*.java)
+
+$(PREVIOUS_REPORT):
+	$(QUIET)$(COPY) src/DiffExample.java.previous src/DiffExample.java
+	$(QUIET)$(call silent_on_success,Testing Differential skips anon class renamings: previous,\
+	$(INFER_BIN) --cost-only -o $(PREVIOUS_DIR) -- $(JAVAC) src/*.java)

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

@@ -0,0 +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}]}

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

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// This class has the following costs:
+// 1 top, 1 bottom, 2 constant (incl. constructor), 1 linear
+public class DiffExample {
+
+    // cost: top
+    private static void top() {
+        int i = 0;
+        while (i >=0) {
+            i++;
+        }
+    }
+
+    // cost: bottom (0)
+    private static void bottom() {}
+
+    // cost: constant (5)
+    private static int constant() {
+        int i, j;
+        i = 17;
+        j = 31;
+
+        return i + j + 3 + 7;
+    }
+
+    // cost: linear
+    private static int linear(int k) {
+        for (int i = 0; i < k; i++) {
+        }
+        return 0;
+    }
+}

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

@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// This class has the following costs:
+// 1 constant (constructor), 1 linear, 1 quadratic
+public class DiffExample {
+    // cost: linear
+    private static int linear(int k) {
+        for (int i = 0; i < k; i++) {
+        }
+        return 0;
+    }
+
+    // cost: quadratic
+    private static void quadratic(int k) {
+        for (int i = 0; i < k; i++) {
+            linear(k);
+        }
+    }
+}

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

@@ -0,0 +1 @@
+{"top":{"current":0,"previous":0},"zero":{"current":0,"previous":0},"degrees":[]}

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

@@ -0,0 +1 @@
+{"top":{"current":0,"previous":0},"zero":{"current":0,"previous":0},"degrees":[]}

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

@@ -0,0 +1 @@
+{"top":{"current":0,"previous":0},"zero":{"current":0,"previous":0},"degrees":[]}

infer/tests/differential.make

@@ -17,6 +17,8 @@ CURRENT_DIR = infer-out-current
 PREVIOUS_DIR = infer-out-previous
 CURRENT_REPORT = $(CURRENT_DIR)/report.json
 PREVIOUS_REPORT = $(PREVIOUS_DIR)/report.json
+CURRENT_COSTS = $(CURRENT_DIR)/costs-report.json
+PREVIOUS_COSTS = $(PREVIOUS_DIR)/costs-report.json
 
 default: analyze
 
@@ -35,6 +37,7 @@ $(EXPECTED_TEST_OUTPUT): $(CURRENT_REPORT) $(PREVIOUS_REPORT) $(MODIFIED_FILES_F
 	$(QUIET)$(call silent_on_success,Computing results difference in $(TEST_REL_DIR),\
 	  $(INFER_BIN) -o $(INFER_OUT) --project-root $(CURDIR) reportdiff \
 		--report-current $(CURRENT_REPORT) --report-previous $(PREVIOUS_REPORT) \
+		--costs-current $(CURRENT_COSTS) --costs-previous $(PREVIOUS_COSTS) \
 		$(DIFFERENTIAL_ARGS))
 	$(QUIET)$(INFER_BIN) report -o $(INFER_OUT) \
 		--issues-fields $(INFERPRINT_ISSUES_FIELDS) \
@@ -48,6 +51,8 @@ $(EXPECTED_TEST_OUTPUT): $(CURRENT_REPORT) $(PREVIOUS_REPORT) $(MODIFIED_FILES_F
 		--issues-fields $(INFERPRINT_ISSUES_FIELDS) \
 		--from-json-report $(INFER_OUT)/differential/preexisting.json \
 		--issues-tests preexisting.exp.test
+	$(QUIET)$(COPY) $(INFER_OUT)/differential/costs_summary.json \
+		costs_summary.json.exp.test
 
 .PHONY: print
 print: $(EXPECTED_TEST_OUTPUT)
@@ -57,12 +62,14 @@ test: print
 	$(QUIET)$(call check_no_diff,introduced.exp,introduced.exp.test)
 	$(QUIET)$(call check_no_diff,fixed.exp,fixed.exp.test)
 	$(QUIET)$(call check_no_diff,preexisting.exp,preexisting.exp.test)
+	$(QUIET)$(call check_no_diff,costs_summary.json.exp,costs_summary.json.exp.test)
 
 .PHONY: replace
 replace: $(EXPECTED_TEST_OUTPUT)
 	cp introduced.exp.test introduced.exp
 	cp fixed.exp.test fixed.exp
 	cp preexisting.exp.test preexisting.exp
+	cp costs_summary.json.exp.test costs_summary.json.exp
 
 .PHONY: clean
 clean: