From 14d4f862ebb5250700e8de5f1eeba5996a37fbdd Mon Sep 17 00:00:00 2001 From: jrm Date: Wed, 17 Feb 2016 09:33:18 -0800 Subject: [PATCH] @Performance critical checker: compute the expensive call stack lazily when reporting errors Summary:public Before this diff, the checker was collecting in a bottom-up fashion all possible call trees from `PerforamanceCritical`-annotated methods to `Expensive`-annotated ones. With this diff, we just collect the names of the direct transitively expensive callees and compute the expensive call stacks when reporting errors only. Reviewed By: sblackshear Differential Revision: D2938635 fb-gh-sync-id: dcdd13c shipit-source-id: dcdd13c --- infer/src/backend/callTree.ml | 27 ------ infer/src/backend/callTree.mli | 18 ---- infer/src/backend/specs.ml | 4 +- infer/src/backend/specs.mli | 5 +- infer/src/checkers/performanceCritical.ml | 90 ++++++++++--------- .../java/checkers/ExpensiveCallExample.java | 5 +- 6 files changed, 59 insertions(+), 90 deletions(-) delete mode 100644 infer/src/backend/callTree.ml delete mode 100644 infer/src/backend/callTree.mli diff --git a/infer/src/backend/callTree.ml b/infer/src/backend/callTree.ml deleted file mode 100644 index 94d3c110d..000000000 --- a/infer/src/backend/callTree.ml +++ /dev/null @@ -1,27 +0,0 @@ -(* - * Copyright (c) 2015 - present Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - *) - - -module F = Format - -type call = Procname.t * Location.t - -type t = - | Direct of call - | Indirect of call * t list - - -let pp fmt tree = - let rec loop stack = function - | Direct (pname, _) -> - F.fprintf fmt "%s -> %s" stack (Procname.to_string pname) - | Indirect ((pname, _), l) -> - let stack' = stack ^ " -> " ^ (Procname.to_string pname) in - IList.iter (loop stack') l in - loop "@" tree diff --git a/infer/src/backend/callTree.mli b/infer/src/backend/callTree.mli deleted file mode 100644 index 611865f4f..000000000 --- a/infer/src/backend/callTree.mli +++ /dev/null @@ -1,18 +0,0 @@ -(* - * Copyright (c) 2015 - present Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - *) - -type call = Procname.t * Location.t - -(** data-structure to represent call stacks in a compact tree form *) -type t = - | Direct of call - | Indirect of call * t list - -(** print the list of call stacks in the tree *) -val pp : Format.formatter -> t -> unit diff --git a/infer/src/backend/specs.ml b/infer/src/backend/specs.ml index 93c4dea56..eabdc07dc 100644 --- a/infer/src/backend/specs.ml +++ b/infer/src/backend/specs.ml @@ -299,12 +299,14 @@ type phase = FOOTPRINT | RE_EXECUTION type dependency_map_t = int Procname.Map.t +type call = Procname.t * Location.t + (** Payload: results of some analysis *) type payload = { preposts : NormSpec.t list option; (** list of specs *) typestate : unit TypeState.t option; (** final typestate *) - calls: CallTree.t list option; + calls: call list option; } type summary = diff --git a/infer/src/backend/specs.mli b/infer/src/backend/specs.mli index 95d748a10..1b0db6080 100644 --- a/infer/src/backend/specs.mli +++ b/infer/src/backend/specs.mli @@ -114,12 +114,15 @@ type phase = FOOTPRINT | RE_EXECUTION type dependency_map_t = int Procname.Map.t +(** Type for calls consiting in the name of the callee and the location of the call *) +type call = Procname.t * Location.t + (** Payload: results of some analysis *) type payload = { preposts : NormSpec.t list option; (** list of specs *) typestate : unit TypeState.t option; (** final typestate *) - calls: CallTree.t list option; (** list of call tree *) + calls: call list option; (** list of call tree *) } (** Procedure summary *) diff --git a/infer/src/checkers/performanceCritical.ml b/infer/src/checkers/performanceCritical.ml index f06c549c8..659f84b70 100644 --- a/infer/src/checkers/performanceCritical.ml +++ b/infer/src/checkers/performanceCritical.ml @@ -78,39 +78,42 @@ let method_is_expensive tenv pname = || check_method Annotations.ia_is_expensive pname -let lookup_call_trees pname = +let lookup_expensive_calls pname = match Specs.get_summary pname with | None -> [] | Some summary -> begin match summary.Specs.payload.Specs.calls with - | Some tree -> tree + | Some calls -> calls | None -> [] end +let method_calls_expensive tenv pname = + method_is_expensive tenv pname + || lookup_expensive_calls pname <> [] + + let lookup_location pname = match Specs.get_summary pname with | None -> Location.dummy | Some summary -> summary.Specs.attributes.ProcAttributes.loc -let collect_expensive_call tenv caller_pdesc checked_pnames call_trees (pname, _) = - if Procname.Set.mem pname !checked_pnames then call_trees +let collect_expensive_call tenv caller_pdesc checked_pnames call_list (pname, _) = + if Procname.Set.mem pname !checked_pnames then call_list else begin Ondemand.do_analysis caller_pdesc pname; checked_pnames := Procname.Set.add pname !checked_pnames; let call_loc = lookup_location pname in - if method_is_expensive tenv pname then - (CallTree.Direct (pname, call_loc)) :: call_trees + if method_calls_expensive tenv pname then + (pname, call_loc) :: call_list else - match lookup_call_trees pname with - | [] -> call_trees - | calls -> (CallTree.Indirect ((pname, call_loc), calls)) :: call_trees + call_list end -let update_summary call_trees pname = +let update_summary calls pname = match Specs.get_summary pname with | None -> () | Some summary -> @@ -118,12 +121,12 @@ let update_summary call_trees pname = { summary with Specs.payload = { summary.Specs.payload with - Specs.calls = Some call_trees; } + Specs.calls = Some calls; } } in Specs.add_summary pname updated_summary -let report_expensive_calls pname pdesc loc call_trees = +let report_expensive_calls tenv pname pdesc loc calls = let string_of_pname = Procname.to_simplified_string ~withclass:true in let update_trace trace loc = if Location.equal loc Location.dummy then trace @@ -135,30 +138,36 @@ let report_expensive_calls pname pdesc loc call_trees = lt_node_tags = []; } in trace_elem :: trace in - let rec report_call_tree (trace, stack_str) call_tree = - match call_tree with - | CallTree.Direct (expensive_pname, callee_loc) -> - let final_trace = IList.rev (update_trace trace callee_loc) in - let exp_pname_str = string_of_pname expensive_pname in - let description = - Printf.sprintf - "Method `%s` annotated with `@%s` calls `%s%s` where `%s` is annotated with `@%s`" - (Procname.to_simplified_string pname) - Annotations.performance_critical - stack_str - exp_pname_str - exp_pname_str - Annotations.expensive in - let exn = - Exceptions.Checkers (calls_expensive_method, Localise.verbatim_desc description) in - Reporting.log_error pname ~loc: (Some loc) ~ltr: (Some final_trace) exn - | CallTree.Indirect ((callee_pname, callee_loc), sub_trees) -> - let callee_pname_str = string_of_pname callee_pname in - let new_stack_str = stack_str ^ callee_pname_str ^ " -> " in - let new_trace = update_trace trace callee_loc in - IList.iter (report_call_tree (new_trace, new_stack_str)) sub_trees in + let rec report_expensive_call visited_pnames (trace, stack_str) (callee_pname, callee_loc) = + if method_is_expensive tenv callee_pname then + let final_trace = IList.rev (update_trace trace callee_loc) in + let exp_pname_str = string_of_pname callee_pname in + let description = + Printf.sprintf + "Method `%s` annotated with `@%s` calls `%s%s` where `%s` is annotated with `@%s`" + (Procname.to_simplified_string pname) + Annotations.performance_critical + stack_str + exp_pname_str + exp_pname_str + Annotations.expensive in + let exn = + Exceptions.Checkers (calls_expensive_method, Localise.verbatim_desc description) in + Reporting.log_error pname ~loc: (Some loc) ~ltr: (Some final_trace) exn + else + let next_calls = lookup_expensive_calls callee_pname in + let callee_pname_str = string_of_pname callee_pname in + let new_stack_str = stack_str ^ callee_pname_str ^ " -> " in + let new_trace = update_trace trace callee_loc in + let unseen_pnames, updated_visited = + IList.fold_left + (fun (accu, set) (p, loc) -> + if Procname.Set.mem p set then (accu, set) + else ((p, loc) :: accu, Procname.Set.add p set)) + ([], visited_pnames) next_calls in + IList.iter (report_expensive_call updated_visited (new_trace, new_stack_str)) unseen_pnames in let start_trace = update_trace [] (Cfg.Procdesc.get_loc pdesc) in - IList.iter (report_call_tree (start_trace, "")) call_trees + IList.iter (report_expensive_call Procname.Set.empty (start_trace, "")) calls let check_one_procedure tenv pname pdesc = @@ -186,18 +195,15 @@ let check_one_procedure tenv pname pdesc = PatternMatch.proc_iter_overridden_methods check_expensive_subtyping_rules tenv pname; - let expensive_call_trees = + let expensive_calls = let checked_pnames = ref Procname.Set.empty in Cfg.Procdesc.fold_calls (collect_expensive_call tenv pdesc checked_pnames) [] pdesc in - update_summary expensive_call_trees pname; + update_summary expensive_calls pname; - match expensive_call_trees with - | [] -> () - | call_trees when performance_critical -> - report_expensive_calls pname pdesc loc call_trees - | _ -> () + if performance_critical then + report_expensive_calls tenv pname pdesc loc expensive_calls let callback_performance_checker { Callbacks.proc_desc; proc_name; get_proc_desc; tenv } = diff --git a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java index 79d26cfce..39a909de6 100644 --- a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java +++ b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java @@ -45,7 +45,10 @@ public class ExpensiveCallExample implements AnnotatedInterface { void nonExpensiveMethod() {} @Expensive - void expensiveMethod() {} + void expensiveMethod() { + // The checker should still report the expensive call stack despite the call cycle + methodWrapper(); + } void methodWrapper() { expensiveMethod();