Skip to content

Commit f6cc06f

Browse files
mheibermeta-codesync[bot]
authored andcommitted
Remove hh_client --save-state and hh_server --save-state
Summary: ## What Remove the ability to generate saved states via `hh_client --save-state` and `hh_server --save-state` command-line options. This includes: - Removing CLI argument parsing for --save-state and --gen-saved-ignore-type-errors - Removing the SAVE_STATE RPC command and its handler - Removing save_state functions from SaveStateService - Deleting integration tests that exercised this functionality ## Why Saved state generation in prod is done via hh_distc, so, imo, these old mechanisms for making saved states are pure risk and overhead–I suspect that over time the behavior of unused duplicate APIs diverges, producing footguns. andrewjkennedy raised the concern that being able to make saved states *only* with hh_distc may tie us to remote execution: iuc this we're safe, since hh_distc can be run locally-only, without remote execution. ## Follow-on Add more testing for making saved states with hh_distc: D91029946 Reviewed By: madgen Differential Revision: D91018321 fbshipit-source-id: 6a8618da8e08c741a4ffde8aded80e479807eac6
1 parent 6794b0c commit f6cc06f

22 files changed

Lines changed: 13 additions & 825 deletions

hphp/hack/src/client/clientArgs.ml

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ let parse_check_args cmd ~from_default : ClientEnv.client_check_env =
144144
let from = ref from_default in
145145
let show_spinner = ref None in
146146
let show_tast = ref false in
147-
let gen_saved_ignore_type_errors = ref false in
148147
let ignore_hh_version = ref false in
149148
let save_64bit = ref None in
150149
let save_human_readable_64bit_dep_map = ref None in
@@ -414,10 +413,6 @@ let parse_check_args cmd ~from_default : ClientEnv.client_check_env =
414413
end,
415414
" (mode) for each entry in input list get list of function dependencies [file:line:character list]"
416415
);
417-
( "--gen-saved-ignore-type-errors",
418-
Arg.Set gen_saved_ignore_type_errors,
419-
" generate a saved state even if there are type errors (default: false)."
420-
);
421416
( "--get-method-name",
422417
Arg.String (fun x -> set_mode (MODE_IDENTIFY_SYMBOL3 x)),
423418
(* alias for --identify-function *) "" );
@@ -669,10 +664,6 @@ rewrite to the function names to something like `foo_1` and `foo_2`.
669664
Arg.String (fun x -> set_mode (MODE_SAVE_NAMING x)),
670665
" (mode) Save the naming table to the given file."
671666
^ " Returns the number of files and symbols written to disk." );
672-
( "--save-state",
673-
Arg.String (fun x -> set_mode (MODE_SAVE_STATE x)),
674-
" (mode) Save a saved state to the given file."
675-
^ " Returns number of edges dumped from memory to the database." );
676667
( "--save-64bit",
677668
Arg.String (fun x -> save_64bit := Some x),
678669
" save discovered 64-bit to the given directory" );
@@ -899,7 +890,6 @@ rewrite to the function names to something like `foo_1` and `foo_2`.
899890
force_dormant_start = !force_dormant_start;
900891
from = !from;
901892
show_spinner = Option.value ~default:is_interactive !show_spinner;
902-
gen_saved_ignore_type_errors = !gen_saved_ignore_type_errors;
903893
ignore_hh_version = !ignore_hh_version;
904894
saved_state_ignore_hhconfig = !saved_state_ignore_hhconfig;
905895
paths;

hphp/hack/src/client/clientCheck.ml

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,6 @@ open Ocaml_overrides
1212
module SyntaxTree =
1313
Full_fidelity_syntax_tree.WithSyntax (Full_fidelity_positioned_syntax)
1414

15-
module SaveStateResultPrinter = ClientResultPrinter.Make (struct
16-
type t = SaveStateServiceTypes.save_state_result
17-
18-
let to_string t =
19-
Printf.sprintf
20-
"Dependency table edges added: %d"
21-
t.SaveStateServiceTypes.dep_table_edges_added
22-
23-
let to_json (t : SaveStateServiceTypes.save_state_result) =
24-
Hh_json.JSON_Object (ServerError.get_save_state_result_props_json t)
25-
end)
26-
2715
module SaveNamingResultPrinter = ClientResultPrinter.Make (struct
2816
type t = SaveStateServiceTypes.save_naming_result
2917

@@ -138,7 +126,6 @@ let connect
138126
custom_telemetry_data;
139127
preexisting_warnings;
140128
error_format = _;
141-
gen_saved_ignore_type_errors = _;
142129
paths = _;
143130
max_errors = _;
144131
mode = _;
@@ -822,18 +809,6 @@ let main_internal
822809
in
823810
SaveNamingResultPrinter.go result args.output_json;
824811
Lwt.return (Exit_status.No_error, telemetry)
825-
| ClientEnv.MODE_SAVE_STATE path ->
826-
let () = Sys_utils.mkdir_p (Filename.dirname path) in
827-
(* Convert to real path because Client and Server may have
828-
* different cwd and we want to use the client processes' cwd. *)
829-
let path = Path.make path in
830-
let%lwt (result, telemetry) =
831-
rpc args
832-
@@ ServerCommandTypes.SAVE_STATE
833-
(Path.to_string path, args.gen_saved_ignore_type_errors)
834-
in
835-
SaveStateResultPrinter.go result args.output_json;
836-
Lwt.return (Exit_status.No_error, telemetry)
837812
| ClientEnv.MODE_SEARCH query ->
838813
if not (String.equal query "this_is_just_to_check_liveness_of_hh_server")
839814
then begin

hphp/hack/src/client/clientCheckStatus.ml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ let go status error_format ~is_interactive ~output_json ~max_errors =
6363
~output_json
6464
~error_format
6565
~error_list
66-
~save_state_result:None
6766
~recheck_stats:last_recheck_stats;
6867
0
6968
) else

hphp/hack/src/client/clientEnv.ml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ type client_mode =
6262
| MODE_REWRITE_DECLARATIONS
6363
| MODE_REWRITE_LAMBDA_PARAMETERS of string list
6464
| MODE_SAVE_NAMING of string
65-
| MODE_SAVE_STATE of string
6665
| MODE_SEARCH of string
6766
| MODE_SERVER_RAGE
6867
| MODE_STATS
@@ -92,7 +91,6 @@ type client_check_env = {
9291
force_dormant_start: bool;
9392
from: string;
9493
show_spinner: bool;
95-
gen_saved_ignore_type_errors: bool;
9694
ignore_hh_version: bool;
9795
saved_state_ignore_hhconfig: bool;
9896
paths: string list;

hphp/hack/src/client/clientSavedStateProjectMetadata.ml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ let main (env : ClientEnv.client_check_env) (config : ServerLocalConfig.t) :
3232
force_dormant_start = _;
3333
from = _;
3434
show_spinner = _;
35-
gen_saved_ignore_type_errors = _;
3635
paths = _;
3736
max_errors = _;
3837
preexisting_warnings = _;

hphp/hack/src/client_and_server/saveStateService.ml

Lines changed: 0 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -101,145 +101,15 @@ let get_hot_classes (filename : string) : SSet.t =
101101
|> List.map ~f:Hh_json.get_string_exn
102102
|> SSet.of_list
103103

104-
(** Dumps the naming-table (a saveable form of FileInfo), and errors if any,
105-
and hot class decls. *)
106-
let dump_naming_and_errors
107-
(output_filename : string)
108-
(naming_table : Naming_table.t)
109-
(errors : Errors.t) : unit =
110-
let naming_sql_filename = output_filename ^ "_naming.sql" in
111-
let (save_result : Naming_sqlite.save_result) =
112-
Naming_table.save naming_table naming_sql_filename
113-
in
114-
Hh_logger.log
115-
"Inserted symbols into the naming table:\n%s"
116-
(Naming_sqlite.show_save_result save_result);
117-
Hh_logger.log
118-
"Finished saving naming table with %d errors."
119-
(List.length save_result.Naming_sqlite.errors);
120-
121-
if List.length save_result.Naming_sqlite.errors > 0 then
122-
Exit.exit Exit_status.Sql_assertion_failure;
123-
124-
assert (Sys.file_exists naming_sql_filename);
125-
Hh_logger.log "Saved naming table sqlite to '%s'" naming_sql_filename;
126-
(* Let's not write empty error files. *)
127-
(if Errors.is_empty errors then
128-
()
129-
else
130-
let error_files : saved_state_errors = Errors.get_failed_files errors in
131-
save_contents (get_errors_filename output_filename) error_files);
132-
()
133-
134-
(** Sorts and dumps the error relative paths in JSON format.
135-
* An empty JSON list will be dumped if there are no errors.*)
136-
let dump_errors_json (output_filename : string) (errors : Errors.t) : unit =
137-
let error_files = Errors.get_failed_files errors in
138-
let errors_json =
139-
Hh_json.(
140-
JSON_Array
141-
(List.rev
142-
(Relative_path.Set.fold
143-
~init:[]
144-
~f:(fun relative_path acc ->
145-
JSON_String (Relative_path.suffix relative_path) :: acc)
146-
error_files)))
147-
in
148-
let chan = Stdlib.open_out (get_errors_filename_json output_filename) in
149-
Hh_json.json_to_output chan errors_json;
150-
Stdlib.close_out chan
151-
152104
let saved_state_info_file_name ~base_file_name = base_file_name ^ "_info.json"
153105

154-
let saved_state_build_revision_write ~(base_file_name : string) : unit =
155-
let info_file = saved_state_info_file_name ~base_file_name in
156-
let open Hh_json in
157-
Out_channel.with_file info_file ~f:(fun fh ->
158-
json_to_output fh
159-
@@ JSON_Object [("build_revision", string_ Build_id.build_revision)])
160-
161106
let saved_state_build_revision_read ~(base_file_name : string) : string =
162107
let info_file = saved_state_info_file_name ~base_file_name in
163108
let contents = RealDisk.cat info_file in
164109
let json = Some (Hh_json.json_of_string contents) in
165110
let build_revision = Hh_json_helpers.Jget.string_exn json "build_revision" in
166111
build_revision
167112

168-
let dump_dep_graph_64bit ~mode ~db_name ~incremental_info_file =
169-
let t = Unix.gettimeofday () in
170-
let base_dep_graph =
171-
match mode with
172-
| Typing_deps_mode.InMemoryMode base_dep_graph -> base_dep_graph
173-
| Typing_deps_mode.SaveToDiskMode { graph; _ } -> graph
174-
in
175-
let () =
176-
let open Hh_json in
177-
Out_channel.with_file incremental_info_file ~f:(fun fh ->
178-
json_to_output fh
179-
@@ JSON_Object [("base_dep_graph", string_opt base_dep_graph)])
180-
in
181-
let dep_table_edges_added =
182-
Typing_deps.save_discovered_edges
183-
mode
184-
~dest:db_name
185-
~reset_state_after_saving:false
186-
in
187-
let (_ : float) = Hh_logger.log_duration "Writing discovered edges took" t in
188-
{ dep_table_edges_added }
189-
190-
(** Saves the saved state to the given path. Returns number of dependency
191-
* edges dumped into the database. *)
192-
let save_state (env : ServerEnv.env) (output_filename : string) :
193-
save_state_result =
194-
let () = Sys_utils.mkdir_p (Filename.dirname output_filename) in
195-
let db_name =
196-
match env.ServerEnv.deps_mode with
197-
| Typing_deps_mode.InMemoryMode _
198-
| Typing_deps_mode.SaveToDiskMode _ ->
199-
output_filename ^ "_64bit_dep_graph.delta"
200-
in
201-
let () =
202-
if Sys.file_exists output_filename then
203-
failwith
204-
(Printf.sprintf "Cowardly refusing to overwrite '%s'." output_filename)
205-
else
206-
()
207-
in
208-
let () =
209-
if Sys.file_exists db_name then
210-
failwith (Printf.sprintf "Cowardly refusing to overwrite '%s'." db_name)
211-
else
212-
()
213-
in
214-
let (_ : float) =
215-
let naming_table = env.ServerEnv.naming_table in
216-
let errors = env.ServerEnv.errorl in
217-
let t = Unix.gettimeofday () in
218-
dump_naming_and_errors output_filename naming_table errors;
219-
Hh_logger.log_duration "Saving saved-state naming+errors took" t
220-
in
221-
match env.ServerEnv.deps_mode with
222-
| Typing_deps_mode.InMemoryMode _ ->
223-
let incremental_info_file = output_filename ^ "_incremental_info.json" in
224-
dump_errors_json output_filename env.ServerEnv.errorl;
225-
saved_state_build_revision_write ~base_file_name:output_filename;
226-
dump_dep_graph_64bit
227-
~mode:env.ServerEnv.deps_mode
228-
~db_name
229-
~incremental_info_file
230-
| Typing_deps_mode.SaveToDiskMode
231-
{ graph = _; new_edges_dir; human_readable_dep_map_dir } ->
232-
dump_errors_json output_filename env.ServerEnv.errorl;
233-
saved_state_build_revision_write ~base_file_name:output_filename;
234-
Hh_logger.warn
235-
"saveStateService: not saving 64-bit dep graph edges to disk, because they are already in %s"
236-
new_edges_dir;
237-
(match human_readable_dep_map_dir with
238-
| None -> ()
239-
| Some dir ->
240-
Hh_logger.warn "saveStateService: human readable dep map dir: %s" dir);
241-
{ dep_table_edges_added = 0 }
242-
243113
let go_naming (naming_table : Naming_table.t) (output_filename : string) :
244114
(save_naming_result, string) result =
245115
Utils.try_with_stack (fun () ->
@@ -257,10 +127,3 @@ let go_naming (naming_table : Naming_table.t) (output_filename : string) :
257127
nt_symbols_added = save_result.Naming_sqlite.symbols_added;
258128
})
259129
|> Result.map_error ~f:(fun e -> Exception.get_ctor_string e)
260-
261-
(* If successful, returns the # of edges from the dependency table that were written. *)
262-
(* TODO: write some other stats, e.g., the number of names, the number of errors, etc. *)
263-
let go (env : ServerEnv.env) (output_filename : string) :
264-
(save_state_result, string) result =
265-
Utils.try_with_stack (fun () -> save_state env output_filename)
266-
|> Result.map_error ~f:(fun e -> Exception.get_ctor_string e)

0 commit comments

Comments
 (0)