From 944176bf674aced87ea6e1e4ae1ef79d9e77793f Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Fri, 13 May 2016 05:48:55 -0700 Subject: [PATCH] load json config files lazily and at most once Summary: The checkers check was causing perf issues because it kept loading the json of inferconfig. To prevent this from happening again, load json files inside config.ml, and only export `Yojson.Basic.json Lazy.t` values to other modules. Also move the list of checks disabled by default into config.ml for better discoverability. Reviewed By: jberdine Differential Revision: D3293041 fbshipit-source-id: 4a38b26 --- infer/lib/python/inferlib/jwlib.py | 6 ++ infer/src/backend/CommandLineOption.ml | 6 +- infer/src/backend/config.ml | 39 ++++++- infer/src/backend/config.mli | 5 +- infer/src/backend/inferconfig.ml | 107 +++++++------------ infer/src/backend/inferconfig.mli | 10 +- infer/src/backend/utils.ml | 12 +++ infer/src/backend/utils.mli | 6 ++ infer/src/checkers/annotationReachability.ml | 3 +- infer/src/java/jAnnotation.ml | 14 +-- infer/src/java/jMain.ml | 5 +- 11 files changed, 114 insertions(+), 99 deletions(-) diff --git a/infer/lib/python/inferlib/jwlib.py b/infer/lib/python/inferlib/jwlib.py index 22539137a..b5b645399 100644 --- a/infer/lib/python/inferlib/jwlib.py +++ b/infer/lib/python/inferlib/jwlib.py @@ -277,6 +277,12 @@ class AnalyzerWithFrontendWrapper(analyze.AnalyzerWrapper): if not self.args.absolute_paths: infer_cmd += ['-project_root', self.args.project_root] + if os.path.isfile(self.javac.suppress_warnings_out) and \ + os.path.getsize(self.javac.suppress_warnings_out) == 0: + with codecs.open(self.javac.suppress_warnings_out, 'w', + encoding=config.CODESET) as json_file: + json_file.write('{}') + infer_cmd += [ '-results_dir', self.args.infer_out, '-verbose_out', self.javac.verbose_out, diff --git a/infer/src/backend/CommandLineOption.ml b/infer/src/backend/CommandLineOption.ml index 9170853b7..a44bffae8 100644 --- a/infer/src/backend/CommandLineOption.ml +++ b/infer/src/backend/CommandLineOption.ml @@ -262,14 +262,14 @@ let mk_symbol_opt ~symbols ?(deprecated=[]) ~long ?short ?exes ?(meta="") doc = ~mk_spec:(fun set -> Arg.Symbol (strings, set)) let mk_symbol_seq ?(default=[]) ~symbols ?(deprecated=[]) ~long ?short ?exes ?(meta="") doc = - let strings = IList.map fst symbols in let sym_to_str = IList.map (fun (x,y) -> (y,x)) symbols in let of_string str = IList.assoc string_equal str symbols in let to_string sym = IList.assoc ( = ) sym sym_to_str in mk ~deprecated ~long ?short ~default ?exes ~meta:(" ,-separated sequence" ^ meta) doc ~default_to_string:(fun syms -> String.concat " " (IList.map to_string syms)) - ~mk_setter:(fun var str_seq -> var := IList.map of_string (Str.split (Str.regexp ",") str_seq)) - ~mk_spec:(fun set -> Arg.Symbol (strings, set)) + ~mk_setter:(fun var str_seq -> + var := IList.map of_string (Str.split (Str.regexp_string ",") str_seq)) + ~mk_spec:(fun set -> Arg.String set) let anon_fun = ref (fun arg -> raise (Arg.Bad ("unexpected anonymous argument: " ^ arg))) diff --git a/infer/src/backend/config.ml b/infer/src/backend/config.ml index 49266a800..5668f46ed 100644 --- a/infer/src/backend/config.ml +++ b/infer/src/backend/config.ml @@ -58,6 +58,10 @@ let bound_error_allowed_in_procedure_call = true let captured_dir_name = "captured" +let checks_disabled_by_default = [ + "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL"; +] + let default_failure_name = "ASSERTION_FAILURE" let default_in_zip_results_dir = "infer" @@ -134,6 +138,8 @@ let specs_files_suffix = ".specs" let start_filename = ".start" +let suppress_warnings_annotations_long = "suppress-warnings-annotations" + (** If true performs taint analysis *) let taint_analysis = true @@ -627,7 +633,7 @@ and subtype_multirange = "Use the multirange subtyping domain" and suppress_warnings_out = - CLOpt.mk_string_opt ~deprecated:["suppress_warnings_out"] ~long:"suppress-warnings-annotations" + CLOpt.mk_string_opt ~deprecated:["suppress_warnings_out"] ~long:suppress_warnings_annotations_long ~exes:CLOpt.[J] ~meta:"path" "Path to list of collected @SuppressWarnings annotations" (** command line flag: if true, produce a svg file *) @@ -962,7 +968,6 @@ and dotty_cfg_libs = !dotty_cfg_libs and eradicate = !eradicate and err_file_cmdline = !err_file and infer_cache = !infer_cache -and inferconfig_home = !inferconfig_home and iterations = !iterations and javac_verbose_out = !verbose_out and join_cond = !join_cond @@ -1006,7 +1011,6 @@ and spec_abs_level = !spec_abs_level and specs_library = !specs_library and stats_mode = !stats and subtype_multirange = !subtype_multirange -and suppress_warnings_annotations = !suppress_warnings_out and svg = !svg and symops_per_iteration = !symops_per_iteration and test = !test @@ -1023,3 +1027,32 @@ and write_dotty = !write_dotty and write_html = !write_html and xml_specs = !xml_specs and zip_libraries = !zip_libraries + +let inferconfig_json = + let inferconfig_path = + match !inferconfig_home, project_root with + | Some dir, _ | _, Some dir -> Filename.concat dir inferconfig_file + | None, None -> inferconfig_file in + lazy ( + match read_optional_json_file inferconfig_path with + | Ok json -> json + | Error msg -> + F.fprintf F.err_formatter "Could not read or parse Infer config in %s:@\n%s@." + inferconfig_path msg; + exit 1) + +and suppress_warnings_json = lazy ( + let error msg = + F.fprintf F.err_formatter "There was an issue reading the option %s.@\n" + suppress_warnings_annotations_long ; + F.fprintf F.err_formatter "If you did not call %s directly, this is likely a bug in Infer.@\n" + (Filename.basename Sys.executable_name) ; + F.fprintf F.err_formatter "%s@." msg ; + exit 1 in + match !suppress_warnings_out with + | Some path -> ( + match read_optional_json_file path with + | Ok json -> json + | Error msg -> error ("Could not read or parse the supplied " ^ path ^ ":\n" ^ msg)) + | None -> + error ("Error: The option " ^ suppress_warnings_annotations_long ^ " was not provided")) diff --git a/infer/src/backend/config.mli b/infer/src/backend/config.mli index 5e0154d92..7fc51be9f 100644 --- a/infer/src/backend/config.mli +++ b/infer/src/backend/config.mli @@ -38,6 +38,7 @@ val assign : string val attributes_dir_name : string val backend_stats_dir_name : string val bound_error_allowed_in_procedure_call : bool +val checks_disabled_by_default : string list val captured_dir_name : string val default_failure_name : string val default_in_zip_results_dir : string @@ -123,7 +124,7 @@ val dotty_cfg_libs : bool val eradicate : bool val err_file_cmdline : string val infer_cache : string option -val inferconfig_home : string option +val inferconfig_json : Yojson.Basic.json Lazy.t val iterations : int val javac_verbose_out : string val join_cond : int @@ -168,7 +169,7 @@ val spec_abs_level : int val specs_library : string list val stats_mode : bool val subtype_multirange : bool -val suppress_warnings_annotations : string option +val suppress_warnings_json : Yojson.Basic.json Lazy.t val svg : bool val symops_per_iteration : int val test : bool diff --git a/infer/src/backend/inferconfig.ml b/infer/src/backend/inferconfig.ml index 5f24fc67f..1ab116cea 100644 --- a/infer/src/backend/inferconfig.ml +++ b/infer/src/backend/inferconfig.ml @@ -13,9 +13,11 @@ module L = Logging (** Look up a key in a json file containing a list of strings *) let lookup_string_list key json = - Yojson.Basic.Util.filter_member key [json] - |> Yojson.Basic.Util.flatten - |> Yojson.Basic.Util.filter_string + try + Yojson.Basic.Util.filter_member key [json] + |> Yojson.Basic.Util.flatten + |> Yojson.Basic.Util.filter_string + with Yojson.Basic.Util.Type_error _ -> [] type path_filter = DB.source_file -> bool type error_filter = Localise.t -> bool @@ -145,11 +147,11 @@ let rec translate json_key accu (json : Yojson.Basic.json) : pattern list = | _ -> assert false (* Creates a list of matching patterns for the given inferconfig file *) -let load_patterns json_key inferconfig = +let load_patterns json_key json = let found = Yojson.Basic.Util.filter_member json_key - [Yojson.Basic.from_file inferconfig] in + [json] in IList.fold_left (translate json_key) [] found @@ -200,7 +202,7 @@ end module type Matcher = sig type matcher = DB.source_file -> Procname.t -> bool - val load_matcher : string -> matcher + val load_matcher : Yojson.Basic.json Lazy.t -> matcher end (* Module to create matcher based on source file names or class names and method names *) @@ -212,7 +214,6 @@ struct let default_matcher : matcher = fun _ _ -> false - let create_method_matcher m_patterns = if m_patterns = [] then default_matcher @@ -260,12 +261,8 @@ struct fun source_file proc_name -> m_matcher source_file proc_name || s_matcher source_file proc_name - let load_matcher inferconfig = - if Sys.file_exists inferconfig then - create_file_matcher (load_patterns M.json_key inferconfig) - else - default_matcher - + let load_matcher json = + create_file_matcher (load_patterns M.json_key (Lazy.force json)) let _pp_pattern fmt pattern = let pp_string fmt s = @@ -301,19 +298,15 @@ struct type matcher = (string -> bool) -> Procname.t -> bool - let default_matcher _ _ = false - - let load_matcher inferconfig = - if Sys.file_exists inferconfig then - fun is_subtype proc_name -> - let is_matching = function - | Method_pattern (language, mp) -> - is_subtype mp.class_name - && Option.map_default (match_method language proc_name) false mp.method_name - | _ -> failwith "Expecting method pattern" in - IList.exists is_matching (load_patterns M.json_key inferconfig) - else - default_matcher + let load_matcher json = + let patterns = load_patterns M.json_key (Lazy.force json) in + fun is_subtype proc_name -> + let is_matching = function + | Method_pattern (language, mp) -> + is_subtype mp.class_name + && Option.map_default (match_method language proc_name) false mp.method_name + | _ -> failwith "Expecting method pattern" in + IList.exists is_matching patterns end @@ -333,32 +326,17 @@ module ModeledExpensiveMatcher = OverridesMatcher(struct let json_key = "modeled_expensive" end) -let disabled_checks_by_default = [ - "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL" -] - -let inferconfig () = - match Config.inferconfig_home, Config.project_root with - | Some dir, _ | _, Some dir -> Filename.concat dir Config.inferconfig_file - | None, None -> Config.inferconfig_file - let load_filters analyzer = - let inferconfig_file = inferconfig () in - if Sys.file_exists inferconfig_file then - try - let json = Yojson.Basic.from_file inferconfig_file in - let inferconfig = - { - whitelist = lookup_string_list (analyzer ^ "_whitelist") json; - blacklist = lookup_string_list (analyzer ^ "_blacklist") json; - blacklist_files_containing = - lookup_string_list (analyzer ^ "_blacklist_files_containing") json; - suppress_errors = lookup_string_list (analyzer ^ "_suppress_errors") json; - } in - Some inferconfig - with Sys_error _ -> None - else None - + let lazy json = Config.inferconfig_json in + let inferconfig = + { + whitelist = lookup_string_list (analyzer ^ "_whitelist") json; + blacklist = lookup_string_list (analyzer ^ "_blacklist") json; + blacklist_files_containing = + lookup_string_list (analyzer ^ "_blacklist_files_containing") json; + suppress_errors = lookup_string_list (analyzer ^ "_suppress_errors") json; + } in + Some inferconfig let filters_from_inferconfig inferconfig : filters = let path_filter = @@ -394,20 +372,17 @@ let create_filters analyzer = (* Decide whether a checker or error type is enabled or disabled based on*) (* white/black listing in .inferconfig and the default value *) -let is_checker_enabled checker_name = +let is_checker_enabled = let black_listed_checks = lazy ( - try - lookup_string_list "disable_checks" - (Yojson.Basic.from_file (inferconfig ())) - with _ -> []) in + lookup_string_list "disable_checks" (Lazy.force Config.inferconfig_json)) in let white_listed_checks = lazy ( - try - lookup_string_list "enable_checks" - (Yojson.Basic.from_file (inferconfig ())) - with _ -> []) in - match IList.mem (=) checker_name (Lazy.force black_listed_checks), IList.mem (=) checker_name (Lazy.force white_listed_checks) with + lookup_string_list "enable_checks" (Lazy.force Config.inferconfig_json)) in + (* return a closure so that we automatically memoize the json lookups thanks to lazy *) + function checker_name -> + match IList.mem (=) checker_name (Lazy.force black_listed_checks), + IList.mem (=) checker_name (Lazy.force white_listed_checks) with | false, false -> (* if it's not amond white/black listed then we use default value *) - not (IList.mem (=) checker_name disabled_checks_by_default) + not (IList.mem (=) checker_name Config.checks_disabled_by_default) | true, false -> (* if it's blacklisted and not whitelisted then it should be disabled *) false | false, true -> (* if it is not blacklisted and it is whitelisted then it should be enabled *) @@ -440,10 +415,4 @@ let test () = (Sys.getcwd ()) let skip_translation_headers = - lazy ( - match - lookup_string_list "skip_translation_headers" - (Yojson.Basic.from_file (inferconfig ())) - with - | exception _ -> [] - | headers -> headers) + lazy (lookup_string_list "skip_translation_headers" (Lazy.force Config.inferconfig_json)) diff --git a/infer/src/backend/inferconfig.mli b/infer/src/backend/inferconfig.mli index f7b727758..99ff63d63 100644 --- a/infer/src/backend/inferconfig.mli +++ b/infer/src/backend/inferconfig.mli @@ -9,9 +9,6 @@ open! Utils -(** get the path to the .inferconfig file *) -val inferconfig : unit -> string - (** Filter type for a source file *) type path_filter = DB.source_file -> bool @@ -36,7 +33,7 @@ val create_filters : analyzer -> filters module type Matcher = sig type matcher = DB.source_file -> Procname.t -> bool - val load_matcher : string -> matcher + val load_matcher : Yojson.Basic.json Lazy.t -> matcher end module NeverReturnNull : Matcher @@ -47,7 +44,7 @@ module SuppressWarningsMatcher : Matcher module ModeledExpensiveMatcher : sig type matcher = (string -> bool) -> Procname.t -> bool - val load_matcher : string -> matcher + val load_matcher : Yojson.Basic.json Lazy.t -> matcher end (** Load the config file and list the files to report on *) @@ -55,5 +52,6 @@ val test: unit -> unit val skip_translation_headers : string list Lazy.t -(* is_checker_enabled error_name is true if error_name is whitelisted in .inferconfig or if it's enabled by default *) +(** is_checker_enabled [error_name] is [true] if [error_name] is whitelisted in .inferconfig or if + it's enabled by default *) val is_checker_enabled : string -> bool diff --git a/infer/src/backend/utils.ml b/infer/src/backend/utils.ml index 61cd6f1c9..3f9154e92 100644 --- a/infer/src/backend/utils.ml +++ b/infer/src/backend/utils.ml @@ -16,6 +16,10 @@ module F = Format functions and builtin equality. Use IList instead. *) module List = struct end +type ('a, 'b) result = + | Ok of 'a + | Error of 'b + (** initial process times *) let initial_times = Unix.times () @@ -563,3 +567,11 @@ let string_append_crc_cutoff ?(cutoff=100) ?(key="") name = let name_for_crc = name ^ key in string_crc_hex32 name_for_crc in name_up_to_cutoff ^ "." ^ crc_str + +let read_optional_json_file path = + if Sys.file_exists path then + try + Ok (Yojson.Basic.from_file path) + with Sys_error msg | Yojson.Json_error msg -> + Error msg + else Ok (`Assoc []) diff --git a/infer/src/backend/utils.mli b/infer/src/backend/utils.mli index 89751b565..31098c46e 100644 --- a/infer/src/backend/utils.mli +++ b/infer/src/backend/utils.mli @@ -16,6 +16,10 @@ functions and builtin equality. Use IList instead. *) module List : sig end +type ('a, 'b) result = + | Ok of 'a + | Error of 'b + (** initial process times *) val initial_times : Unix.process_times @@ -249,3 +253,5 @@ val analyzers: analyzer list val string_of_analyzer: analyzer -> string val analyzer_of_string: string -> analyzer + +val read_optional_json_file : string -> (Yojson.Basic.json, string) result diff --git a/infer/src/checkers/annotationReachability.ml b/infer/src/checkers/annotationReachability.ml index f0f9a746e..246bfcd21 100644 --- a/infer/src/checkers/annotationReachability.ml +++ b/infer/src/checkers/annotationReachability.ml @@ -106,8 +106,7 @@ let expensive_overrides_unexpensive = let is_modeled_expensive = let matcher = - lazy (let config_file = Inferconfig.inferconfig () in - Inferconfig.ModeledExpensiveMatcher.load_matcher config_file) in + lazy (Inferconfig.ModeledExpensiveMatcher.load_matcher Config.inferconfig_json) in fun tenv proc_name -> match proc_name with | Procname.Java proc_name_java -> not (Builtin.is_registered proc_name) && diff --git a/infer/src/java/jAnnotation.ml b/infer/src/java/jAnnotation.ml index dfd60fc24..9a8471007 100644 --- a/infer/src/java/jAnnotation.ml +++ b/infer/src/java/jAnnotation.ml @@ -14,17 +14,9 @@ open Javalib_pack let is_suppress_warnings_annotated = - let matcher = - lazy - (let default_matcher = fun _ -> false in - match Config.suppress_warnings_annotations with - | Some f -> - (try - let m = Inferconfig.SuppressWarningsMatcher.load_matcher f in - (m DB.source_file_empty) - with Yojson.Json_error _ -> - default_matcher) - | None -> failwith "Local config expected!") in + let matcher = lazy ( + Inferconfig.SuppressWarningsMatcher.load_matcher Config.suppress_warnings_json + DB.source_file_empty) in fun proc_name -> (Lazy.force matcher) proc_name diff --git a/infer/src/java/jMain.ml b/infer/src/java/jMain.ml index ceb04d10e..aa4697cc6 100644 --- a/infer/src/java/jMain.ml +++ b/infer/src/java/jMain.ml @@ -136,9 +136,8 @@ let do_all_files classpath sources classes = let tenv = load_tenv () in let linereader = Printer.LineReader.create () in let skip_translation_matcher = - Inferconfig.SkipTranslationMatcher.load_matcher (Inferconfig.inferconfig ()) in - let never_null_matcher = - Inferconfig.NeverReturnNull.load_matcher (Inferconfig.inferconfig ()) in + Inferconfig.SkipTranslationMatcher.load_matcher Config.inferconfig_json in + let never_null_matcher = Inferconfig.NeverReturnNull.load_matcher Config.inferconfig_json in let skip source_file = skip_translation_matcher source_file Procname.empty_block in let translate_source_file basename (package_opt, _) source_file =