From 28b455cbfa578180473746722b1ecfa56dd814e2 Mon Sep 17 00:00:00 2001 From: Varun Arora Date: Mon, 12 Mar 2018 11:03:22 -0700 Subject: [PATCH] [backend] Directly report analysis perf from ProcessPool assigned Task so it works correctly with new parallelism Summary: Previously, `backend_stats` were getting logged correctly only when `infer analyze` was directly called, not `infer run`. Now, we report `backend_stats` directly, as part of the `iterate_callbacks` function in the task passed to the `ProcessPool`. As a side benefit, `aggregated_stats` are also logged correctly now. Reviewed By: dulmarod Differential Revision: D7195525 fbshipit-source-id: fb2a400 --- infer/src/backend/InferAnalyze.ml | 8 ----- infer/src/backend/InferAnalyze.mli | 2 -- infer/src/backend/PerfStats.ml | 32 +++++++++++-------- infer/src/backend/PerfStats.mli | 6 ++-- infer/src/backend/callbacks.ml | 8 +++++ infer/src/backend/infer.ml | 1 - .../buck_flavors_deterministic/Makefile | 2 +- 7 files changed, 32 insertions(+), 27 deletions(-) diff --git a/infer/src/backend/InferAnalyze.ml b/infer/src/backend/InferAnalyze.ml index c23544158..7008778f6 100644 --- a/infer/src/backend/InferAnalyze.ml +++ b/infer/src/backend/InferAnalyze.ml @@ -153,11 +153,3 @@ let main ~changed_files ~makefile = List.iteri ~f:analyze_cluster clusters_to_analyze ; L.progress "@\nAnalysis finished in %as@." Pp.elapsed_time () ) ; output_json_makefile_stats clusters_to_analyze - - -let register_perf_stats_report () = - let cluster = - match Config.cluster_cmdline with Some cl -> "_" ^ Filename.basename cl | None -> "" - in - let filename = F.sprintf "%s%s.json" Config.perf_stats_prefix cluster in - PerfStats.register_report_at_exit filename Config.backend_stats_dir_name diff --git a/infer/src/backend/InferAnalyze.mli b/infer/src/backend/InferAnalyze.mli index 8abbbd337..a88350aa4 100644 --- a/infer/src/backend/InferAnalyze.mli +++ b/infer/src/backend/InferAnalyze.mli @@ -14,5 +14,3 @@ open! IStd val main : changed_files:SourceFile.Set.t option -> makefile:string -> unit (** Given a name of the Makefile to use for multicore analysis, analyze the captured code *) - -val register_perf_stats_report : unit -> unit diff --git a/infer/src/backend/PerfStats.ml b/infer/src/backend/PerfStats.ml index cc90b0d51..81462a6f5 100644 --- a/infer/src/backend/PerfStats.ml +++ b/infer/src/backend/PerfStats.ml @@ -161,7 +161,7 @@ let stats source_file stats_type = (stats, stats_event) -let report_at_exit file source_file stats_type () = +let report file source_file stats_type () = try let stats, stats_event = stats source_file stats_type in let json_stats = to_json stats in @@ -183,15 +183,21 @@ let report_at_exit file source_file stats_type () = (Printexc.get_backtrace ()) -let register_report_at_exit = - let registered_files = String.Table.create ~size:4 () in - fun filename ?source_file dirname -> - let dir = Filename.concat Config.results_dir dirname in - let file = Filename.concat dir filename in - (* take care of not double-registering the same perf stat report *) - if not (Hashtbl.mem registered_files file) then ( - String.Table.set registered_files ~key:file ~data:() ; - if not Config.buck_cache_mode then - Epilogues.register - ~f:(report_at_exit file source_file dirname) - ("stats reporting in " ^ file) ) +let registered_files = ref String.Set.empty + +let handle_report filename ?source_file dirname ~f = + let relative_path = Filename.concat dirname filename in + let absolute_path = Filename.concat Config.results_dir relative_path in + (* make sure to not double register the same perf stat report *) + if not (String.Set.mem !registered_files relative_path) then ( + registered_files := String.Set.add !registered_files relative_path ; + f (report absolute_path source_file dirname) ) + + +let report_now filename ?source_file dirname = + handle_report filename ?source_file dirname ~f:(fun report_f -> report_f ()) + + +let register_report_at_exit filename ?source_file dirname = + handle_report filename ?source_file dirname ~f:(fun report_f -> + Epilogues.register ~f:report_f ("stats reporting in " ^ Filename.concat dirname filename) ) diff --git a/infer/src/backend/PerfStats.mli b/infer/src/backend/PerfStats.mli index 1aa07e06c..28cdfad38 100644 --- a/infer/src/backend/PerfStats.mli +++ b/infer/src/backend/PerfStats.mli @@ -17,6 +17,8 @@ val from_json : Yojson.Basic.json -> perf_stats val aggregate : perf_stats list -> Yojson.Basic.json +val report_now : string -> ?source_file:SourceFile.t -> string -> unit +(** Create performance report immediately *) + val register_report_at_exit : string -> ?source_file:SourceFile.t -> string -> unit -(** Create performance report when the current process terminates. Automatically disabled when - [Config.buck_cache_mode] is true. *) +(** Create performance report when the current process terminates *) diff --git a/infer/src/backend/callbacks.ml b/infer/src/backend/callbacks.ml index 5976d7c02..f3309ace7 100644 --- a/infer/src/backend/callbacks.ml +++ b/infer/src/backend/callbacks.ml @@ -118,6 +118,12 @@ let dump_duplicate_procs (exe_env: Exe_env.t) procs = if not (List.is_empty duplicate_procs) then output_to_file duplicate_procs +let create_perf_stats_report source_file = + let abbrev_source_file = DB.source_file_encoding source_file in + let filename = F.sprintf "%s_%s.json" Config.perf_stats_prefix abbrev_source_file in + PerfStats.report_now filename ~source_file Config.backend_stats_dir_name + + (** Invoke all procedure and cluster callbacks on a given environment. *) let iterate_callbacks (exe_env: Exe_env.t) = let saved_language = !Language.curr_language in @@ -146,6 +152,8 @@ let iterate_callbacks (exe_env: Exe_env.t) = List.iter ~f:analyze_proc_name procs_to_analyze ; (* Invoke cluster callbacks. *) iterate_cluster_callbacks procs_to_analyze exe_env get_proc_desc ; + (* Perf logging needs to remain at the end - after analysis work is complete *) + create_perf_stats_report exe_env.source_file ; (* Unregister callbacks *) Ondemand.unset_callbacks () ; Language.curr_language := saved_language diff --git a/infer/src/backend/infer.ml b/infer/src/backend/infer.ml index 67211980c..1b0b61e81 100644 --- a/infer/src/backend/infer.ml +++ b/infer/src/backend/infer.ml @@ -128,7 +128,6 @@ let () = F.fprintf fmt "of cluster %s" (Filename.basename cluster) in L.environment_info "Starting analysis %a" pp_cluster_opt Config.cluster_cmdline ; - InferAnalyze.register_perf_stats_report () ; Driver.analyze_and_report Analyze ~changed_files:(Driver.read_config_changed_files ()) | Report -> InferPrint.main ~report_json:None diff --git a/infer/tests/build_systems/buck_flavors_deterministic/Makefile b/infer/tests/build_systems/buck_flavors_deterministic/Makefile index daf53a9d1..bb5af76a7 100644 --- a/infer/tests/build_systems/buck_flavors_deterministic/Makefile +++ b/infer/tests/build_systems/buck_flavors_deterministic/Makefile @@ -12,7 +12,7 @@ ANALYZER = checkers BUCK_TARGET = //src:hello SOURCES = $(wildcard src/hello.c) OBJECTS = buck-out/gen/src/hello\#compile-hello.c.o1f717d69,default/hello.c.o -INFER_OPTIONS = --report-custom-error --developer-mode --project-root $(TESTS_DIR) +INFER_OPTIONS = --report-custom-error --project-root $(TESTS_DIR) INFERPRINT_OPTIONS = --project-root $(TESTS_DIR) --issues-tests CLEAN_EXTRA = buck-out capture_hash-1.sha capture_hash-2.sha