From 6874926b10b96ce32dd436bcb918469a3afd7441 Mon Sep 17 00:00:00 2001 From: Phoebe Nichols Date: Fri, 30 Aug 2019 09:55:27 -0700 Subject: [PATCH] Clean specs directory before running capture Summary: Currently, the specs directory is cleaned after running capture. This means that the `changed-files` are interpreted in the context of the second set of source files. Therefore if a procedure is deleted from the second set of source files, its specs file will not be deleted. This moves the cleaning of the specs directory to before capture, to avoid this problem. Reviewed By: ngorogiannis Differential Revision: D17093980 fbshipit-source-id: e1a8d8a54 --- infer/src/backend/InferAnalyze.ml | 63 ++++++++++--------- infer/src/backend/InferAnalyze.mli | 3 + infer/src/infer.ml | 1 + .../costs_summary.json.exp | 2 +- .../reverse_analysis_callgraph.dot | 11 ++-- .../src_after/Test.java | 4 ++ .../reverse_analysis_callgraph.dot | 8 +-- 7 files changed, 53 insertions(+), 39 deletions(-) diff --git a/infer/src/backend/InferAnalyze.ml b/infer/src/backend/InferAnalyze.ml index 6c8bd3610..21aaae8e5 100644 --- a/infer/src/backend/InferAnalyze.ml +++ b/infer/src/backend/InferAnalyze.ml @@ -142,29 +142,38 @@ let analyze source_files_to_analyze = let invalidate_changed_procedures changed_files = - L.progress "Incremental analysis: invalidating procedures that have been changed@." ; - let reverse_callgraph = CallGraph.create CallGraph.default_initial_capacity in - ReverseAnalysisCallGraph.build reverse_callgraph ; - let total_nodes = CallGraph.n_procs reverse_callgraph in - SourceFile.Set.iter - (fun sf -> - SourceFiles.proc_names_of_source sf - |> List.iter ~f:(CallGraph.flag_reachable reverse_callgraph) ) - changed_files ; - if Config.debug_level_analysis > 0 then - CallGraph.to_dotty reverse_callgraph "reverse_analysis_callgraph.dot" ; - let invalidated_nodes = - CallGraph.fold_flagged reverse_callgraph - ~f:(fun node acc -> SpecsFiles.delete node.pname ; acc + 1) - 0 - in - L.progress - "Incremental analysis: %d nodes in reverse analysis call graph, %d of which were invalidated @." - total_nodes invalidated_nodes ; - ScubaLogging.log_count ~label:"incremental_analysis.total_nodes" ~value:total_nodes ; - ScubaLogging.log_count ~label:"incremental_analysis.invalidated_nodes" ~value:invalidated_nodes ; - (* save some memory *) - CallGraph.reset reverse_callgraph + if Config.incremental_analysis then ( + let changed_files = + match changed_files with + | Some cf -> + cf + | None -> + L.die InternalError "Incremental analysis enabled without specifying changed files" + in + L.progress "Incremental analysis: invalidating procedures that have been changed@." ; + let reverse_callgraph = CallGraph.create CallGraph.default_initial_capacity in + ReverseAnalysisCallGraph.build reverse_callgraph ; + let total_nodes = CallGraph.n_procs reverse_callgraph in + SourceFile.Set.iter + (fun sf -> + SourceFiles.proc_names_of_source sf + |> List.iter ~f:(CallGraph.flag_reachable reverse_callgraph) ) + changed_files ; + if Config.debug_level_analysis > 0 then + CallGraph.to_dotty reverse_callgraph "reverse_analysis_callgraph.dot" ; + let invalidated_nodes = + CallGraph.fold_flagged reverse_callgraph + ~f:(fun node acc -> SpecsFiles.delete node.pname ; acc + 1) + 0 + in + L.progress + "Incremental analysis: %d nodes in reverse analysis call graph, %d of which were \ + invalidated @." + total_nodes invalidated_nodes ; + ScubaLogging.log_count ~label:"incremental_analysis.total_nodes" ~value:total_nodes ; + ScubaLogging.log_count ~label:"incremental_analysis.invalidated_nodes" ~value:invalidated_nodes ; + (* save some memory *) + CallGraph.reset reverse_callgraph ) let main ~changed_files = @@ -173,13 +182,7 @@ let main ~changed_files = L.progress "Invalidating procedures to be reanalyzed@." ; Summary.OnDisk.reset_all ~filter:(Lazy.force Filtering.procedures_filter) () ; L.progress "Done@." ) - else if Config.incremental_analysis then - match changed_files with - | Some cf -> - invalidate_changed_procedures cf - | None -> - L.die InternalError "Incremental analysis enabled without specifying changed files" - else DB.Results_dir.clean_specs_dir () ; + else if not Config.incremental_analysis then DB.Results_dir.clean_specs_dir () ; let source_files = get_source_files_to_analyze ~changed_files in (* empty all caches to minimize the process heap to have less work to do when forking *) clear_caches () ; diff --git a/infer/src/backend/InferAnalyze.mli b/infer/src/backend/InferAnalyze.mli index c1f7bb54b..ef636a5e4 100644 --- a/infer/src/backend/InferAnalyze.mli +++ b/infer/src/backend/InferAnalyze.mli @@ -12,3 +12,6 @@ open! IStd val main : changed_files:SourceFile.Set.t option -> unit (** Given a name of the Makefile to use for multicore analysis, analyze the captured code *) + +val invalidate_changed_procedures : SourceFile.Set.t option -> unit +(** Invalidate specs files for procedures that have changed. Used for incremental analysis. *) diff --git a/infer/src/infer.ml b/infer/src/infer.ml index 399da4033..13ce732ba 100644 --- a/infer/src/infer.ml +++ b/infer/src/infer.ml @@ -17,6 +17,7 @@ let run driver_mode = let open Driver in run_prologue driver_mode ; let changed_files = read_config_changed_files () in + InferAnalyze.invalidate_changed_procedures changed_files ; capture driver_mode ~changed_files ; analyze_and_report driver_mode ~changed_files ; run_epilogue () diff --git a/infer/tests/build_systems/incremental_analysis_cost_change/costs_summary.json.exp b/infer/tests/build_systems/incremental_analysis_cost_change/costs_summary.json.exp index bd944faed..9de511cee 100644 --- a/infer/tests/build_systems/incremental_analysis_cost_change/costs_summary.json.exp +++ b/infer/tests/build_systems/incremental_analysis_cost_change/costs_summary.json.exp @@ -1 +1 @@ -{"top":{"current":0,"previous":0},"zero":{"current":6,"previous":6},"degrees":[{"degree":0,"current":3,"previous":4},{"degree":100,"current":3,"previous":1},{"degree":200,"current":0,"previous":1}]} \ No newline at end of file +{"top":{"current":0,"previous":0},"zero":{"current":7,"previous":6},"degrees":[{"degree":0,"current":3,"previous":4},{"degree":100,"current":4,"previous":1},{"degree":200,"current":0,"previous":1}]} \ No newline at end of file diff --git a/infer/tests/build_systems/incremental_analysis_cost_change/reverse_analysis_callgraph.dot b/infer/tests/build_systems/incremental_analysis_cost_change/reverse_analysis_callgraph.dot index db2b8e8db..9f6ca4302 100644 --- a/infer/tests/build_systems/incremental_analysis_cost_change/reverse_analysis_callgraph.dot +++ b/infer/tests/build_systems/incremental_analysis_cost_change/reverse_analysis_callgraph.dot @@ -5,21 +5,24 @@ digraph callgraph { N5 [ label = "void Test.main(java.lang.String[])", flag = true ]; N0 [ label = "Object.()", flag = false ]; - N0 -> N6 ; + N0 -> N7 ; N0 -> N1 ; - N7 [ label = "void PrintStream.println(String)", flag = false ]; - N7 -> N2 ; + N7 [ label = "Unchanged.()", flag = false ]; - N6 [ label = "Unchanged.()", flag = false ]; + N6 [ label = "void Test.newMethod(int)", flag = true ]; N4 [ label = "void Test.complexityIncrease(int)", flag = true ]; N4 -> N5 ; N2 [ label = "void Unchanged.orderN(int)", flag = false ]; + N2 -> N6 ; N2 -> N4 ; N2 -> N3 ; + N8 [ label = "void PrintStream.println(String)", flag = false ]; + N8 -> N2 ; + N3 [ label = "void Test.complexityDecrease(int)", flag = true ]; N3 -> N5 ; diff --git a/infer/tests/build_systems/incremental_analysis_cost_change/src_after/Test.java b/infer/tests/build_systems/incremental_analysis_cost_change/src_after/Test.java index ab3ad42cb..f47953a69 100644 --- a/infer/tests/build_systems/incremental_analysis_cost_change/src_after/Test.java +++ b/infer/tests/build_systems/incremental_analysis_cost_change/src_after/Test.java @@ -7,6 +7,10 @@ class Test { + public static void newMethod(int n) { + Unchanged.orderN(n); + } + public static void complexityDecrease(int n) { Unchanged.orderN(n); } diff --git a/infer/tests/build_systems/incremental_analysis_remove_file/reverse_analysis_callgraph.dot b/infer/tests/build_systems/incremental_analysis_remove_file/reverse_analysis_callgraph.dot index ff7207314..92c837938 100644 --- a/infer/tests/build_systems/incremental_analysis_remove_file/reverse_analysis_callgraph.dot +++ b/infer/tests/build_systems/incremental_analysis_remove_file/reverse_analysis_callgraph.dot @@ -1,13 +1,13 @@ digraph callgraph { - N1 [ label = "b", flag = true ]; + N1 [ label = "b", flag = false ]; - N0 [ label = "c", flag = true ]; + N0 [ label = "c", flag = false ]; N0 -> N1 ; - N2 [ label = "a", flag = true ]; + N2 [ label = "a", flag = false ]; N2 -> N3 ; - N3 [ label = "main", flag = true ]; + N3 [ label = "main", flag = false ]; }