[cost] Revert "Use type of Provider.get"

Summary: Type is not enough to say a function call of `Provider.get` is expensive or not.

Reviewed By: jvillard

Differential Revision: D20366206

fbshipit-source-id: 83d3e8741
master
Sungkeun Cho 5 years ago committed by Facebook Github Bot
parent 83062c0499
commit 117ab1164b

@ -10,7 +10,7 @@ module F = Format
module L = Logging
(** invariant: if [package = Some str] then [not (String.equal str "")] *)
type t = {classname: string; package: string option} [@@deriving compare, equal]
type t = {classname: string; package: string option} [@@deriving compare]
let from_string str =
match String.rsplit2 str ~on:'.' with
@ -47,6 +47,3 @@ let is_external_via_config t =
let pp_with_verbosity ~verbose fmt t =
if verbose then pp fmt t else F.pp_print_string fmt (classname t)
let java_lang_integer = {classname= "Integer"; package= Some "java.lang"}

@ -7,7 +7,7 @@
open! IStd
type t [@@deriving compare, equal]
type t [@@deriving compare]
val from_string : string -> t
@ -24,5 +24,3 @@ val classname : t -> string
val is_external_via_config : t -> bool
(** Considered external based on config flags. *)
val java_lang_integer : t

@ -22,11 +22,10 @@ module ModelEnv = struct
; node_hash: int
; location: Location.t
; tenv: Tenv.t
; integer_type_widths: Typ.IntegerWidths.t
; get_cast_type: CastType.get_cast_type option }
; integer_type_widths: Typ.IntegerWidths.t }
let mk_model_env pname ~node_hash ?get_cast_type location tenv integer_type_widths =
{pname; node_hash; location; tenv; integer_type_widths; get_cast_type}
let mk_model_env pname ~node_hash location tenv integer_type_widths =
{pname; node_hash; location; tenv; integer_type_widths}
end
module Exec = struct

@ -16,17 +16,10 @@ module ModelEnv : sig
; node_hash: int
; location: Location.t
; tenv: Tenv.t
; integer_type_widths: Typ.IntegerWidths.t
; get_cast_type: CastType.get_cast_type option }
; integer_type_widths: Typ.IntegerWidths.t }
val mk_model_env :
Procname.t
-> node_hash:int
-> ?get_cast_type:CastType.get_cast_type
-> Location.t
-> Tenv.t
-> Typ.IntegerWidths.t
-> model_env
Procname.t -> node_hash:int -> Location.t -> Tenv.t -> Typ.IntegerWidths.t -> model_env
end
module Exec : sig

@ -1,103 +0,0 @@
(*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*)
open! IStd
module CFG = ProcCfg.Backward (ProcCfg.NormalOneInstrPerNode)
module F = Format
module Loc = struct
type t = Ident of Ident.t | Pvar of Pvar.t [@@deriving compare]
let pp f = function Ident id -> Ident.pp f id | Pvar pvar -> Pvar.pp Pp.text f pvar
end
module Val = struct
include AbstractDomain.FiniteSet (struct
include Typ
let pp = pp Pp.text
end)
let is_integer_type =
let is_integer_type = function
| Typ.{desc= Tstruct (JavaClass name)} ->
JavaClassName.(equal name java_lang_integer)
| _ ->
false
in
fun x -> (not (is_empty x)) && for_all is_integer_type x
end
module Dom = struct
include AbstractDomain.Map (Loc) (Val)
let lookup l m = Option.value (find_opt l m) ~default:Val.bottom
let lookup_ident id m = lookup (Loc.Ident id) m
let lookup_pvar pvar m = lookup (Loc.Pvar pvar) m
let cast id typ m =
let f = function None -> Some (Val.singleton typ) | Some prev -> Some (Val.add typ prev) in
update (Loc.Ident id) f m
let load id pvar m = add (Loc.Pvar pvar) (lookup_ident id m) m
let store pvar id m = add (Loc.Ident id) (lookup_pvar pvar m) m
end
module TransferFunctions = struct
module CFG = CFG
module Domain = Dom
type extras = unit
let exec_instr mem _pdata _node = function
| Sil.Load {id; e= Exp.Lvar pvar} ->
Dom.load id pvar mem
| Sil.Store {e1= Exp.Lvar pvar; e2= Exp.Var id} ->
Dom.store pvar id mem
| Sil.Call (_, Const (Cfun callee_pname), (Exp.Var id, _) :: (Exp.Sizeof {typ}, _) :: _, _, _)
when Procname.equal callee_pname BuiltinDecl.__cast ->
Dom.cast id typ mem
| _ ->
mem
let pp_session_name node f = F.fprintf f "Cast type analysis %a" CFG.Node.pp_id (CFG.Node.id node)
end
module Analyzer = AbstractInterpreter.MakeWTO (TransferFunctions)
let compute_invariant_map summary tenv =
let pdata = ProcData.make summary tenv () in
Analyzer.exec_pdesc ~do_narrowing:false ~initial:Dom.bottom pdata
type get_cast_type = Ident.t -> Val.t
let compute_get_cast_type =
let cache_get, cache_set = Procname.UnitCache.create () in
fun {Callbacks.exe_env; summary} ->
let pname = Summary.get_proc_name summary in
let casted_types =
match cache_get pname with
| Some casted_types ->
casted_types
| None ->
let pdesc = Summary.get_proc_desc summary in
let cfg = CFG.from_pdesc pdesc in
let tenv = Exe_env.get_tenv exe_env pname in
let inv_map = compute_invariant_map summary tenv in
let exit_node_id = CFG.exit_node cfg |> CFG.Node.id in
let casted_types =
Analyzer.extract_post exit_node_id inv_map |> Option.value ~default:Dom.bottom
in
cache_set pname casted_types ; casted_types
in
fun id -> Dom.lookup_ident id casted_types

@ -1,18 +0,0 @@
(*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*)
open! IStd
module Val : sig
type t
val is_integer_type : t -> bool
end
type get_cast_type = Ident.t -> Val.t
val compute_get_cast_type : Callbacks.proc_callback_args -> Ident.t -> Val.t

@ -29,8 +29,7 @@ type extras_WorstCaseCost =
{ inferbo_invariant_map: BufferOverrunAnalysis.invariant_map
; integer_type_widths: Typ.IntegerWidths.t
; get_node_nb_exec: Node.id -> BasicCost.t
; get_callee_summary_and_formals: Procname.t -> callee_summary_and_formals option
; get_cast_type: CastType.get_cast_type }
; get_callee_summary_and_formals: Procname.t -> callee_summary_and_formals option }
let instantiate_cost integer_type_widths ~inferbo_caller_mem ~callee_pname ~callee_formals ~params
~callee_cost ~loc =
@ -59,7 +58,7 @@ module InstrBasicCost = struct
List.exists allocation_functions ~f:(fun f -> Procname.equal callee_pname f)
let get_instr_cost_record tenv ({get_cast_type} as extras) instr_node instr =
let get_instr_cost_record tenv extras instr_node instr =
match instr with
| Sil.Call (ret, Exp.Const (Const.Cfun callee_pname), params, _, _) ->
let {inferbo_invariant_map; integer_type_widths; get_callee_summary_and_formals} = extras in
@ -79,8 +78,8 @@ module InstrBasicCost = struct
| Some model ->
let node_hash = InstrCFG.Node.hash instr_node in
let model_env =
BufferOverrunUtils.ModelEnv.mk_model_env callee_pname ~node_hash ~get_cast_type
loc tenv integer_type_widths
BufferOverrunUtils.ModelEnv.mk_model_env callee_pname ~node_hash loc tenv
integer_type_widths
in
CostDomain.of_operation_cost (model model_env ~ret inferbo_mem)
| None -> (
@ -315,13 +314,9 @@ let compute_get_node_nb_exec node_cfg bound_map : get_node_nb_exec =
let compute_worst_case_cost tenv integer_type_widths get_callee_summary_and_formals instr_cfg_wto
inferbo_invariant_map get_node_nb_exec get_cast_type =
inferbo_invariant_map get_node_nb_exec =
let extras =
{ inferbo_invariant_map
; integer_type_widths
; get_node_nb_exec
; get_callee_summary_and_formals
; get_cast_type }
{inferbo_invariant_map; integer_type_widths; get_node_nb_exec; get_callee_summary_and_formals}
in
WorstCaseCost.compute tenv extras instr_cfg_wto
@ -334,7 +329,7 @@ let report_errors ~is_on_ui_thread proc_desc astate summary =
Check.check_and_report ~is_on_ui_thread astate proc_desc summary
let checker ({Callbacks.exe_env; summary} as callback_args) : Summary.t =
let checker {Callbacks.exe_env; summary} : Summary.t =
let proc_name = Summary.get_proc_name summary in
let tenv = Exe_env.get_tenv exe_env proc_name in
let integer_type_widths = Exe_env.get_integer_type_widths exe_env proc_name in
@ -367,7 +362,6 @@ let checker ({Callbacks.exe_env; summary} as callback_args) : Summary.t =
in
let is_on_ui_thread = ConcurrencyModels.runs_on_ui_thread ~attrs_of_pname tenv proc_name in
let get_node_nb_exec = compute_get_node_nb_exec node_cfg bound_map in
let get_cast_type = CastType.compute_get_cast_type callback_args in
let astate =
let get_callee_summary_and_formals callee_pname =
Payload.read_full ~caller_summary:summary ~callee_pname
@ -377,7 +371,7 @@ let checker ({Callbacks.exe_env; summary} as callback_args) : Summary.t =
let instr_cfg = InstrCFG.from_pdesc proc_desc in
let instr_cfg_wto = InstrCFG.wto instr_cfg in
compute_worst_case_cost tenv integer_type_widths get_callee_summary_and_formals instr_cfg_wto
inferbo_invariant_map get_node_nb_exec get_cast_type
inferbo_invariant_map get_node_nb_exec
in
let () =
let exit_cost_record = astate.WorstCaseCost.costs in

@ -23,13 +23,7 @@ let linear = cost_of_exp ~degree_kind:Polynomials.DegreeKind.Linear
let log = cost_of_exp ~degree_kind:Polynomials.DegreeKind.Log
let provider_get {pname; location; get_cast_type} ~ret:(ret_id, ret_typ) _ : BasicCost.t =
let is_integer_type =
Option.exists get_cast_type ~f:(fun get_cast_type ->
CastType.Val.is_integer_type (get_cast_type ret_id) )
in
if is_integer_type then BasicCost.one
else
let provider_get {pname; location} ~ret:(_, ret_typ) _ : BasicCost.t =
let callsite = CallSite.make pname location in
let path = Symb.SymbolPath.of_callsite ~ret_typ callsite in
let itv = Itv.of_modeled_path ~is_expensive:true path in

@ -1,27 +0,0 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import javax.inject.*;
class ProviderTest {
/* Assume that injecting A is expensive, on the other hand injecting Integer is cheap. */
class A {}
@Inject private Provider<A> mProviderA;
@Inject private Provider<Integer> mProviderInteger;
void use_provided_A(A a) {}
void use_provided_Integer(Integer i) {}
void expensive_get_A_constant() {
use_provided_A(mProviderA.get());
}
void cheap_get_Integer_linear() {
use_provided_Integer(mProviderInteger.get());
}
}

@ -182,7 +182,6 @@ codetoanalyze/java/performance/MathTest.java, codetoanalyze.java.performance.Mat
codetoanalyze/java/performance/MathTest.java, codetoanalyze.java.performance.MathTest.max2_symbolic(int,int):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 6 + 9 ⋅ (max(x, y)), O((max(x, y))), degree = 1,{max(x, y)},Loop at line 20]
codetoanalyze/java/performance/MathTest.java, codetoanalyze.java.performance.MathTest.max_symbolic(int[]):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 2 + 5 ⋅ arr.length + 4 ⋅ (arr.length + 1), O(arr.length), degree = 1,{arr.length + 1},Loop at line 16,{arr.length},Loop at line 16]
codetoanalyze/java/performance/PreconditionTest.java, PreconditionTest.checkNotNull_linear(java.util.ArrayList,java.lang.Object):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 11 + 8 ⋅ list.length + 3 ⋅ (list.length + 1), O(list.length), degree = 1,{list.length + 1},Loop at line 37,{list.length},Loop at line 37]
codetoanalyze/java/performance/ProviderTest.java, ProviderTest.expensive_get_A_constant():void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 3 + (Provider.get()(expensive call).ub), O((Provider.get()(expensive call).ub)), degree = 1,{Provider.get()(expensive call).ub},Modeled call to Provider.get]
codetoanalyze/java/performance/StringBuilderTest.java, StringBuilderTest.append_linear(java.lang.String):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 25 + 5 ⋅ (s.length + 2) + 3 ⋅ (s.length + 3), O(s.length), degree = 1,{s.length + 3},call to void StringBuilderTest.new_linear(String),Loop at line 13,{s.length + 2},call to void StringBuilderTest.new_linear(String),Loop at line 13]
codetoanalyze/java/performance/StringBuilderTest.java, StringBuilderTest.new_linear(java.lang.String):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 9 + 5 ⋅ s.length + 3 ⋅ (s.length + 1), O(s.length), degree = 1,{s.length + 1},Loop at line 13,{s.length},Loop at line 13]
codetoanalyze/java/performance/StringTest.java, StringTest.byte_array_constructor_linear(byte[]):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 6 + 5 ⋅ data.length + 3 ⋅ (data.length + 1), O(data.length), degree = 1,{data.length + 1},Loop at line 55,{data.length},Loop at line 55]

Loading…
Cancel
Save