Skip to content

Commit 156a6bb

Browse files
frank-emrichmeta-codesync[bot]
authored andcommitted
add time_limit_secs option, return partial results when out of time
Summary: This diff adds a `time_limit_secs` option to `hh --find-my-tests-staging`. When reaching the time limit, we return whatever tests we selected up to that point. The JSON output of the command is changed so that instead of just returning a list, we return an object now with a boolean field `out_of_time`, indicating whether we hit the time limit. Note that I'm avoiding the term `timeout` since `hh_client` already has its own, orthogonal timeout mechanism where the client cancels if not getting a response from the server before the timeout. But what I'm doing here is different, since my time limits are enforced by the server, not the client. Implementation notes: On the server side, we check the current time against a deadline value in every iteration of our test BFS graph construction. There is only one exception: `FindRefsService.find_references` can take arbitrarily long, because it may need to type-check thousands of files. Therefore, I added an optional `deadline` parameter to it. There was already a cancelation mechanism within `FindRefsService`, the new `deadline` is re-using that. Reviewed By: madgen Differential Revision: D94660244 fbshipit-source-id: 8d46036bc8c434f6e53e901576062d49b1295af2
1 parent 3771c5d commit 156a6bb

5 files changed

Lines changed: 64 additions & 18 deletions

File tree

hphp/hack/src/client/clientCheck.ml

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,25 @@ let print_refs (results : SearchTypes.Find_refs.absolute list) ~(json : bool) :
4242
else
4343
FindRefsWireFormat.CliHumanReadable.print_results results
4444

45-
let print_find_my_tests_result result ~(json : bool) : unit =
45+
let print_find_my_tests_result_v1 result ~(json : bool) : unit =
4646
let module FMT = ServerCommandTypes.Find_my_tests in
4747
if json then
48-
let result_json = `List (List.map result ~f:FMT.yojson_of_result_entry) in
48+
let result_json =
49+
`List (List.map result ~f:FMT.yojson_of_selected_test_file)
50+
in
4951
print_endline (Yojson.Safe.pretty_to_string result_json)
5052
else
5153
List.iter result ~f:(fun file -> print_endline file.FMT.file_path)
5254

55+
let print_find_my_tests_result result ~(json : bool) : unit =
56+
let module FMT = ServerCommandTypes.Find_my_tests in
57+
if json then
58+
let result_json = FMT.yojson_of_result_data result in
59+
print_endline (Yojson.Safe.pretty_to_string result_json)
60+
else
61+
List.iter result.FMT.selected_test_files ~f:(fun file ->
62+
print_endline file.FMT.file_path)
63+
5364
let parse_name_or_member_id ~name_only_action ~name_and_member_action name =
5465
let pieces = Str.split (Str.regexp "::") name in
5566
let default_namespace str =
@@ -1046,7 +1057,7 @@ let main_internal
10461057
in
10471058
(match result with
10481059
| Ok fmt_result ->
1049-
print_find_my_tests_result fmt_result ~json:args.output_json;
1060+
print_find_my_tests_result_v1 fmt_result ~json:args.output_json;
10501061
Lwt.return (Exit_status.No_error, telemtry)
10511062
| Error error ->
10521063
Printf.eprintf "%s\n" error;

hphp/hack/src/client_and_server/findRefsService.ml

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,18 @@ let fold_one_tast ctx target acc symbol =
255255
| (IGConst cst_name, SO.GConst) -> process_gconst_id cst_name (pos, name)
256256
| _ -> Pos.Map.empty
257257

258-
let should_cancel ~stream_file =
259-
match stream_file with
260-
| None -> false
261-
| Some stream_file -> not (Path.file_exists stream_file)
258+
let should_cancel ~stream_file ~deadline =
259+
let streaming_file_gone =
260+
match stream_file with
261+
| None -> false
262+
| Some stream_file -> not (Path.file_exists stream_file)
263+
in
264+
let past_deadline =
265+
match deadline with
266+
| None -> false
267+
| Some d -> Float.(Unix.gettimeofday () >= d)
268+
in
269+
streaming_file_gone || past_deadline
262270

263271
module Iter = struct
264272
exception Cancelled
@@ -285,10 +293,10 @@ module Iter = struct
285293
}
286294

287295
(** We'll check a few times a second whether we should cancel. *)
288-
let raise_if_should_cancel acc ~t_now ~stream_file =
296+
let raise_if_should_cancel acc ~t_now ~stream_file ~deadline =
289297
let last_cancellation_check =
290298
if Float.(t_now > acc.last_cancellation_check +. 0.2) then begin
291-
if should_cancel ~stream_file then raise Cancelled;
299+
if should_cancel ~stream_file ~deadline then raise Cancelled;
292300
t_now
293301
end else
294302
acc.last_cancellation_check
@@ -347,6 +355,7 @@ let find_refs
347355
(files : Relative_path.t list)
348356
~(omit_declaration : bool)
349357
~(stream_file : Path.t option)
358+
~(deadline : float option)
350359
~(t_start : float) : (SearchTypes.Find_refs.t list, unit) Result.t =
351360
(* The helper function 'results_from_tast' takes a tast, looks at all
352361
use-sites in the tast e.g. "foo(1)" is a use-site of symbol foo,
@@ -406,7 +415,9 @@ let find_refs
406415
let acc =
407416
List.fold files ~init:(Iter.init ()) ~f:(fun acc path ->
408417
let t_now = Unix.gettimeofday () in
409-
let acc = Iter.raise_if_should_cancel ~stream_file ~t_now acc in
418+
let acc =
419+
Iter.raise_if_should_cancel ~stream_file ~deadline ~t_now acc
420+
in
410421
let per_file = results_from_tast (tast_of_file path) in
411422
let acc = Iter.{ acc with results = per_file :: acc.results } in
412423
Iter.stream_file ~per_file ~stream_fd ~t_now ~t_start acc)
@@ -444,10 +455,12 @@ let parallel_find_refs
444455
ctx
445456
~(omit_declaration : bool)
446457
~(stream_file : Path.t option)
458+
~(deadline : float option)
447459
~(t_start : float) =
448460
MultiWorker.call
449461
workers
450-
~job:(find_refs ctx target ~omit_declaration ~stream_file ~t_start)
462+
~job:
463+
(find_refs ctx target ~omit_declaration ~stream_file ~deadline ~t_start)
451464
~neutral:(Ok [])
452465
~merge:(fun output acc ->
453466
match (output, acc) with
@@ -458,7 +471,7 @@ let parallel_find_refs
458471
(* We create the "next" function just once, now; it will dole
459472
out chunks of [files] each time it's asked, below. *)
460473
fun () ->
461-
if should_cancel ~stream_file then
474+
if should_cancel ~stream_file ~deadline then
462475
Bucket.Done
463476
else
464477
next ())
@@ -534,7 +547,8 @@ let get_definitions ctx action =
534547
later time *)
535548
[]
536549

537-
let find_references ctx workers target include_defs files ~stream_file =
550+
let find_references ctx workers target include_defs files ~deadline ~stream_file
551+
=
538552
let len = List.length files in
539553
Hh_logger.debug "find_references: %d files" len;
540554
let t_start = Unix.gettimeofday () in
@@ -549,6 +563,7 @@ let find_references ctx workers target include_defs files ~stream_file =
549563
files
550564
~omit_declaration:true
551565
~stream_file
566+
~deadline
552567
~t_start
553568
else
554569
parallel_find_refs
@@ -558,6 +573,7 @@ let find_references ctx workers target include_defs files ~stream_file =
558573
ctx
559574
~omit_declaration:true
560575
~stream_file
576+
~deadline
561577
~t_start
562578
in
563579
match results with
@@ -588,6 +604,7 @@ let find_references_single_file ctx target file =
588604
[file]
589605
~omit_declaration:false
590606
~stream_file:None
607+
~deadline:None
591608
~t_start:(Unix.gettimeofday ())
592609
in
593610
match results with

hphp/hack/src/client_and_server/findRefsService.mli

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ val find_references :
3535
action_internal ->
3636
bool ->
3737
Relative_path.t list ->
38+
deadline:float option ->
3839
stream_file:Path.t option ->
3940
SearchTypes.Find_refs.t list
4041

hphp/hack/src/client_and_server/serverCommandTypes.ml

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,26 +170,42 @@ module Find_my_tests = struct
170170

171171
type provenance = { root_indices: int list } [@@deriving yojson]
172172

173-
type result_entry = {
173+
type selected_test_file = {
174174
file_path: string;
175175
distance: int;
176176
provenance: provenance option;
177177
}
178178
[@@deriving yojson]
179179

180-
type result = (result_entry list, string) Result.t
180+
(* Types only use by v1 *)
181181

182-
(* Types Used by staging only *)
182+
type result_v1 = (selected_test_file list, string) Result.t
183+
184+
(* Types used by staging only *)
185+
186+
type result_data = {
187+
selected_test_files: selected_test_file list;
188+
out_of_time: bool;
189+
}
190+
[@@deriving yojson]
191+
192+
type result = (result_data, string) Result.t
183193

184194
type config = {
185195
max_distance: int; [@default 1]
186196
max_test_files: int option; [@default None]
187197
root_provenance: bool; [@default false]
198+
time_limit_secs: int option; [@default None]
188199
}
189200
[@@deriving yojson]
190201

191202
let default_config =
192-
{ max_distance = 1; max_test_files = None; root_provenance = false }
203+
{
204+
max_distance = 1;
205+
max_test_files = None;
206+
root_provenance = false;
207+
time_limit_secs = None;
208+
}
193209

194210
type json_input = {
195211
roots: string list;
@@ -467,7 +483,7 @@ type _ t =
467483
-> Find_refs.result_or_retry list t
468484
| FIND_MY_TESTS_V1 :
469485
(int * int option * Find_my_tests.action list)
470-
-> Find_my_tests.result t
486+
-> Find_my_tests.result_v1 t
471487
| FIND_MY_TESTS_STAGING :
472488
(Find_my_tests.config * Find_my_tests.action list)
473489
-> Find_my_tests.result t

hphp/hack/src/client_and_server/serverFindRefs.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ let search ctx target include_defs ~hints ~files ~stream_file genv =
4545
target
4646
include_defs
4747
files
48+
~deadline:None
4849
~stream_file
4950
in
5051
strip_ns res

0 commit comments

Comments
 (0)