From 1d908ff3273a9265501316fec2c657792d44c53d Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Tue, 1 Sep 2020 01:31:22 -0700 Subject: [PATCH] [sqlite] localise statements to where they are used Summary: Move Sqlite registered statements inside the functions that use them, since these are only ever used in one place. Also, reformat some SQL. Also, make `mark_all_source_files_stale` use an un-prepared statement, as it is used at most once per run, wasting resources if registered. Reviewed By: skcho Differential Revision: D23423844 fbshipit-source-id: 07362d19c --- infer/src/IR/Attributes.ml | 94 ++++++++--------- infer/src/IR/Procdesc.ml | 20 ++-- infer/src/base/DBWriter.ml | 163 +++++++++++++++--------------- infer/src/base/ResultsDatabase.ml | 43 ++++---- 4 files changed, 161 insertions(+), 159 deletions(-) diff --git a/infer/src/IR/Attributes.ml b/infer/src/IR/Attributes.ml index b293e190d..336da4f92 100644 --- a/infer/src/IR/Attributes.ml +++ b/infer/src/IR/Attributes.ml @@ -28,59 +28,59 @@ let proc_kind_of_attr (proc_attributes : ProcAttributes.t) = if proc_attributes.is_defined then ProcDefined else ProcUndefined -let replace pname pname_blob akind source_file attributes proc_desc callees = - let pname_str = Procname.to_string pname in - let akind_int64 = int64_of_attributes_kind akind in - let proc_desc_blob = Procdesc.SQLite.serialize proc_desc in - let callees_blob = Procname.SQLiteList.serialize callees in - DBWriter.replace_attributes ~pname_str ~pname:pname_blob ~akind:akind_int64 ~source_file - ~attributes ~proc_desc:proc_desc_blob ~callees:callees_blob - - -let find_more_defined_statement = - ResultsDatabase.register_statement - {| -SELECT attr_kind -FROM procedures -WHERE proc_name = :pname - AND attr_kind > :akind -|} - - -let should_try_to_update pname_blob akind = - ResultsDatabase.with_registered_statement find_more_defined_statement ~f:(fun db find_stmt -> - Sqlite3.bind find_stmt 1 (* :pname *) pname_blob - |> SqliteUtils.check_result_code db ~log:"replace bind pname" ; - Sqlite3.bind find_stmt 2 (* :akind *) (Sqlite3.Data.INT (int64_of_attributes_kind akind)) - |> SqliteUtils.check_result_code db ~log:"replace bind attribute kind" ; - SqliteUtils.result_single_column_option ~finalize:false ~log:"Attributes.replace" db find_stmt - |> (* there is no entry with a strictly larger "definedness" for that proc name *) - Option.is_none ) - - -let select_statement = - ResultsDatabase.register_statement "SELECT proc_attributes FROM procedures WHERE proc_name = :k" - - -let find pname_blob = - ResultsDatabase.with_registered_statement select_statement ~f:(fun db select_stmt -> - Sqlite3.bind select_stmt 1 pname_blob - |> SqliteUtils.check_result_code db ~log:"find bind proc name" ; - SqliteUtils.result_single_column_option ~finalize:false ~log:"Attributes.find" db select_stmt - |> Option.map ~f:ProcAttributes.SQLite.deserialize ) +let should_try_to_update = + let find_more_defined_statement = + ResultsDatabase.register_statement + {| + SELECT attr_kind + FROM procedures + WHERE proc_name = :pname + AND attr_kind > :akind + |} + in + fun pname_blob akind -> + ResultsDatabase.with_registered_statement find_more_defined_statement ~f:(fun db find_stmt -> + Sqlite3.bind find_stmt 1 (* :pname *) pname_blob + |> SqliteUtils.check_result_code db ~log:"replace bind pname" ; + Sqlite3.bind find_stmt 2 (* :akind *) (Sqlite3.Data.INT (int64_of_attributes_kind akind)) + |> SqliteUtils.check_result_code db ~log:"replace bind attribute kind" ; + SqliteUtils.result_single_column_option ~finalize:false ~log:"Attributes.replace" db + find_stmt + |> (* there is no entry with a strictly larger "definedness" for that proc name *) + Option.is_none ) + + +let find = + let select_statement = + ResultsDatabase.register_statement "SELECT proc_attributes FROM procedures WHERE proc_name = :k" + in + fun pname_blob -> + ResultsDatabase.with_registered_statement select_statement ~f:(fun db select_stmt -> + Sqlite3.bind select_stmt 1 pname_blob + |> SqliteUtils.check_result_code db ~log:"find bind proc name" ; + SqliteUtils.result_single_column_option ~finalize:false ~log:"Attributes.find" db + select_stmt + |> Option.map ~f:ProcAttributes.SQLite.deserialize ) let load pname = find (Procname.SQLite.serialize pname) let store ~proc_desc (attr : ProcAttributes.t) = - let pkind = proc_kind_of_attr attr in + let akind = proc_kind_of_attr attr in let key = Procname.SQLite.serialize attr.proc_name in - if should_try_to_update key pkind then - replace attr.proc_name key pkind - (SourceFile.SQLite.serialize attr.loc.Location.file) - (ProcAttributes.SQLite.serialize attr) - proc_desc - (Option.map proc_desc ~f:Procdesc.get_static_callees |> Option.value ~default:[]) + if should_try_to_update key akind then + let source_file_blob = SourceFile.SQLite.serialize attr.loc.Location.file in + let attributes_blob = ProcAttributes.SQLite.serialize attr in + let pname_str = Procname.to_string attr.proc_name in + let akind_int64 = int64_of_attributes_kind akind in + let proc_desc_blob = Procdesc.SQLite.serialize proc_desc in + let callees_blob = + Option.map proc_desc ~f:Procdesc.get_static_callees + |> Option.value ~default:[] |> Procname.SQLiteList.serialize + in + DBWriter.replace_attributes ~pname_str ~pname:key ~akind:akind_int64 + ~source_file:source_file_blob ~attributes:attributes_blob ~proc_desc:proc_desc_blob + ~callees:callees_blob let find_file_capturing_procedure pname = diff --git a/infer/src/IR/Procdesc.ml b/infer/src/IR/Procdesc.ml index a0448cdbb..db3d61ea9 100644 --- a/infer/src/IR/Procdesc.ml +++ b/infer/src/IR/Procdesc.ml @@ -835,13 +835,13 @@ module SQLite = SqliteUtils.MarshalledNullableDataNOTForComparison (struct type nonrec t = t end) -let load_statement = - ResultsDatabase.register_statement "SELECT cfg FROM procedures WHERE proc_name = :k" - - -let load pname = - ResultsDatabase.with_registered_statement load_statement ~f:(fun db stmt -> - Procname.SQLite.serialize pname |> Sqlite3.bind stmt 1 - |> SqliteUtils.check_result_code db ~log:"load bind proc name" ; - SqliteUtils.result_single_column_option ~finalize:false ~log:"Procdesc.load" db stmt - |> Option.bind ~f:SQLite.deserialize ) +let load = + let load_statement = + ResultsDatabase.register_statement "SELECT cfg FROM procedures WHERE proc_name = :k" + in + fun pname -> + ResultsDatabase.with_registered_statement load_statement ~f:(fun db stmt -> + Procname.SQLite.serialize pname |> Sqlite3.bind stmt 1 + |> SqliteUtils.check_result_code db ~log:"load bind proc name" ; + SqliteUtils.result_single_column_option ~finalize:false ~log:"Procdesc.load" db stmt + |> Option.bind ~f:SQLite.deserialize ) diff --git a/infer/src/base/DBWriter.ml b/infer/src/base/DBWriter.ml index dc97e53d8..481279d7d 100644 --- a/infer/src/base/DBWriter.ml +++ b/infer/src/base/DBWriter.ml @@ -10,93 +10,90 @@ module L = Logging module F = Format module Implementation = struct - let attribute_replace_statement = - (* The innermost SELECT returns either the current attributes_kind and source_file associated with - the given proc name, or default values of (-1,""). These default values have the property that - they are always "less than" any legit value. More precisely, MAX ensures that some value is - returned even if there is no row satisfying WHERE (we'll get NULL in that case, the value in - the row otherwise). COALESCE then returns the first non-NULL value, which will be either the - value of the row corresponding to that pname in the DB, or the default if no such row exists. - - The next (second-outermost) SELECT filters out that value if it is "more defined" than the ones - we would like to insert (which will never be the case if the default values are returned). If - not, it returns a trivial row (consisting solely of NULL since we don't use its values) and the - INSERT OR REPLACE will proceed and insert or update the values stored into the DB for that - pname. *) - (* TRICK: use the source file to be more deterministic in case the same procedure name is defined - in several files *) - (* TRICK: older versions of sqlite (prior to version 3.15.0 (2016-10-14)) do not support row - values so the lexicographic ordering for (:akind, :sfile) is done by hand *) - ResultsDatabase.register_statement - {| - INSERT OR REPLACE INTO procedures - SELECT :pname, :proc_name_hum, :akind, :sfile, :pattr, :cfg, :callees - FROM ( - SELECT NULL + let replace_attributes = + let attribute_replace_statement = + (* The innermost SELECT returns either the current attributes_kind and source_file associated with + the given proc name, or default values of (-1,""). These default values have the property that + they are always "less than" any legit value. More precisely, MAX ensures that some value is + returned even if there is no row satisfying WHERE (we'll get NULL in that case, the value in + the row otherwise). COALESCE then returns the first non-NULL value, which will be either the + value of the row corresponding to that pname in the DB, or the default if no such row exists. + + The next (second-outermost) SELECT filters out that value if it is "more defined" than the ones + we would like to insert (which will never be the case if the default values are returned). If + not, it returns a trivial row (consisting solely of NULL since we don't use its values) and the + INSERT OR REPLACE will proceed and insert or update the values stored into the DB for that + pname. *) + (* TRICK: use the source file to be more deterministic in case the same procedure name is defined + in several files *) + (* TRICK: older versions of sqlite (prior to version 3.15.0 (2016-10-14)) do not support row + values so the lexicographic ordering for (:akind, :sfile) is done by hand *) + ResultsDatabase.register_statement + {| + INSERT OR REPLACE INTO procedures + SELECT :pname, :proc_name_hum, :akind, :sfile, :pattr, :cfg, :callees FROM ( - SELECT COALESCE(MAX(attr_kind),-1) AS attr_kind, - COALESCE(MAX(source_file),"") AS source_file - FROM procedures - WHERE proc_name = :pname ) - WHERE attr_kind < :akind - OR (attr_kind = :akind AND source_file <= :sfile) ) - |} - - - let replace_attributes ~pname_str ~pname ~akind ~source_file ~attributes ~proc_desc ~callees = - ResultsDatabase.with_registered_statement attribute_replace_statement ~f:(fun db replace_stmt -> - Sqlite3.bind replace_stmt 1 (* :pname *) pname - |> SqliteUtils.check_result_code db ~log:"replace bind pname" ; - Sqlite3.bind replace_stmt 2 (* :proc_name_hum *) (Sqlite3.Data.TEXT pname_str) - |> SqliteUtils.check_result_code db ~log:"replace bind proc_name_hum" ; - Sqlite3.bind replace_stmt 3 (* :akind *) (Sqlite3.Data.INT akind) - |> SqliteUtils.check_result_code db ~log:"replace bind attribute kind" ; - Sqlite3.bind replace_stmt 4 (* :sfile *) source_file - |> SqliteUtils.check_result_code db ~log:"replace bind source file" ; - Sqlite3.bind replace_stmt 5 (* :pattr *) attributes - |> SqliteUtils.check_result_code db ~log:"replace bind proc attributes" ; - Sqlite3.bind replace_stmt 6 (* :cfg *) proc_desc - |> SqliteUtils.check_result_code db ~log:"replace bind cfg" ; - Sqlite3.bind replace_stmt 7 (* :callees *) callees - |> SqliteUtils.check_result_code db ~log:"replace bind callees" ; - SqliteUtils.result_unit db ~finalize:false ~log:"Attributes.replace" replace_stmt ) - - - let source_file_store_statement = - ResultsDatabase.register_statement - {| - INSERT OR REPLACE INTO source_files - VALUES (:source, :tenv, :integer_type_widths, :proc_names, :freshly_captured) - |} - - - let add_source_file ~source_file ~tenv ~integer_type_widths ~proc_names = - ResultsDatabase.with_registered_statement source_file_store_statement ~f:(fun db store_stmt -> - Sqlite3.bind store_stmt 1 source_file - (* :source *) - |> SqliteUtils.check_result_code db ~log:"store bind source file" ; - Sqlite3.bind store_stmt 2 tenv - (* :tenv *) - |> SqliteUtils.check_result_code db ~log:"store bind type environment" ; - Sqlite3.bind store_stmt 3 integer_type_widths - (* :integer_type_widths *) - |> SqliteUtils.check_result_code db ~log:"store bind integer type widths" ; - Sqlite3.bind store_stmt 4 proc_names - (* :proc_names *) - |> SqliteUtils.check_result_code db ~log:"store bind proc names" ; - Sqlite3.bind store_stmt 5 (Sqlite3.Data.INT Int64.one) - (* :freshly_captured *) - |> SqliteUtils.check_result_code db ~log:"store freshness" ; - SqliteUtils.result_unit ~finalize:false ~log:"Cfg.store" db store_stmt ) - - - let mark_all_source_files_stale_statement = - ResultsDatabase.register_statement "UPDATE source_files SET freshly_captured = 0" + SELECT NULL + FROM ( + SELECT COALESCE(MAX(attr_kind),-1) AS attr_kind, + COALESCE(MAX(source_file),"") AS source_file + FROM procedures + WHERE proc_name = :pname ) + WHERE attr_kind < :akind + OR (attr_kind = :akind AND source_file <= :sfile) ) + |} + in + fun ~pname_str ~pname ~akind ~source_file ~attributes ~proc_desc ~callees -> + ResultsDatabase.with_registered_statement attribute_replace_statement + ~f:(fun db replace_stmt -> + Sqlite3.bind replace_stmt 1 (* :pname *) pname + |> SqliteUtils.check_result_code db ~log:"replace bind pname" ; + Sqlite3.bind replace_stmt 2 (* :proc_name_hum *) (Sqlite3.Data.TEXT pname_str) + |> SqliteUtils.check_result_code db ~log:"replace bind proc_name_hum" ; + Sqlite3.bind replace_stmt 3 (* :akind *) (Sqlite3.Data.INT akind) + |> SqliteUtils.check_result_code db ~log:"replace bind attribute kind" ; + Sqlite3.bind replace_stmt 4 (* :sfile *) source_file + |> SqliteUtils.check_result_code db ~log:"replace bind source file" ; + Sqlite3.bind replace_stmt 5 (* :pattr *) attributes + |> SqliteUtils.check_result_code db ~log:"replace bind proc attributes" ; + Sqlite3.bind replace_stmt 6 (* :cfg *) proc_desc + |> SqliteUtils.check_result_code db ~log:"replace bind cfg" ; + Sqlite3.bind replace_stmt 7 (* :callees *) callees + |> SqliteUtils.check_result_code db ~log:"replace bind callees" ; + SqliteUtils.result_unit db ~finalize:false ~log:"Attributes.replace" replace_stmt ) + + + let add_source_file = + let source_file_store_statement = + ResultsDatabase.register_statement + {| + INSERT OR REPLACE INTO source_files + VALUES (:source, :tenv, :integer_type_widths, :proc_names, :freshly_captured) + |} + in + fun ~source_file ~tenv ~integer_type_widths ~proc_names -> + ResultsDatabase.with_registered_statement source_file_store_statement ~f:(fun db store_stmt -> + Sqlite3.bind store_stmt 1 source_file + (* :source *) + |> SqliteUtils.check_result_code db ~log:"store bind source file" ; + Sqlite3.bind store_stmt 2 tenv + (* :tenv *) + |> SqliteUtils.check_result_code db ~log:"store bind type environment" ; + Sqlite3.bind store_stmt 3 integer_type_widths + (* :integer_type_widths *) + |> SqliteUtils.check_result_code db ~log:"store bind integer type widths" ; + Sqlite3.bind store_stmt 4 proc_names + (* :proc_names *) + |> SqliteUtils.check_result_code db ~log:"store bind proc names" ; + Sqlite3.bind store_stmt 5 (Sqlite3.Data.INT Int64.one) + (* :freshly_captured *) + |> SqliteUtils.check_result_code db ~log:"store freshness" ; + SqliteUtils.result_unit ~finalize:false ~log:"Cfg.store" db store_stmt ) let mark_all_source_files_stale () = - ResultsDatabase.with_registered_statement mark_all_source_files_stale_statement - ~f:(fun db stmt -> SqliteUtils.result_unit db ~finalize:false ~log:"mark_all_stale" stmt) + ResultsDatabase.get_database () + |> SqliteUtils.exec ~stmt:"UPDATE source_files SET freshly_captured = 0" ~log:"mark_all_stale" let merge_procedures_table ~db_file = diff --git a/infer/src/base/ResultsDatabase.ml b/infer/src/base/ResultsDatabase.ml index f5d1bbb6e..e7d372691 100644 --- a/infer/src/base/ResultsDatabase.ml +++ b/infer/src/base/ResultsDatabase.ml @@ -13,26 +13,30 @@ let results_dir_get_path entry = ResultsDirEntryName.get_path ~results_dir:Confi let procedures_schema prefix = Printf.sprintf - {|CREATE TABLE IF NOT EXISTS %sprocedures - ( proc_name TEXT PRIMARY KEY - , proc_name_hum TEXT - , attr_kind INTEGER NOT NULL - , source_file TEXT NOT NULL - , proc_attributes BLOB NOT NULL - , cfg BLOB - , callees BLOB NOT NULL - )|} + {| + CREATE TABLE IF NOT EXISTS %sprocedures + ( proc_name BLOB PRIMARY KEY NOT NULL + , proc_name_hum TEXT + , attr_kind INTEGER NOT NULL + , source_file TEXT NOT NULL + , proc_attributes BLOB NOT NULL + , cfg BLOB + , callees BLOB NOT NULL + ) + |} prefix let source_files_schema prefix = Printf.sprintf - {|CREATE TABLE IF NOT EXISTS %ssource_files - ( source_file TEXT PRIMARY KEY - , type_environment BLOB NOT NULL - , integer_type_widths BLOB - , procedure_names BLOB NOT NULL - , freshly_captured INT NOT NULL )|} + {| + CREATE TABLE IF NOT EXISTS %ssource_files + ( source_file TEXT PRIMARY KEY + , type_environment BLOB NOT NULL + , integer_type_widths BLOB + , procedure_names BLOB NOT NULL + , freshly_captured INT NOT NULL ) + |} prefix @@ -40,10 +44,11 @@ let specs_schema prefix = Printf.sprintf {| CREATE TABLE IF NOT EXISTS %sspecs - ( proc_name TEXT PRIMARY KEY - , analysis_summary BLOB NOT NULL - , report_summary BLOB NOT NULL - )|} + ( proc_name BLOB PRIMARY KEY NOT NULL + , analysis_summary BLOB NOT NULL + , report_summary BLOB NOT NULL + ) + |} prefix