Run typechecker tests in batches instead of one thread at a time

Summary:
This diff implements batching for typechecker tests.

A pet peeve of mine was the inefficiency in our typechecking testing framework, which has been exacerbated by the FFP's parsing regression.

Instead of reparsing all of the builtins every time we run a typechecker test, we take in a list of files, parse all of the builtins *once*, and then typecheck each file separately in its own environment.

In order to do this I had to do a few things:
- AllErrors is no longer its own mode, but a special case of the "errors" mode
- Support typechecking multiple files (previous diff)
- Create a new "batch errors" mode for the typechecker which takes in a list of multiple files, and for each file, generates an ".out" file containing the test results
- Make hh_show use a different out_channel so that hh_show doesn't necessarily print to stderr
- Augment verify.py with a new batch mode that runs test cases in parallel.

The structure before was:
1. verify.py created a 48 worker thread pool that ran each test case separately. Each test case would run hh_single_type_check <filename>.
2. hh_single_typecheck <filename> would return errors as output, and this output was put into .out files
3. verify.py would then check these out files and see if they were equal to the expected files.

The structure now is:
1. verify.py will iterate through the directories provided for each test case, and for each directory, batch test cases by 100.
2. For every chunk of 100 tests, we can call hh_single_type_check --batch-mode <list of files>
3. Because files in the same directory are guaranteed to have the same flags, we can pass the same flags to hh_single_type_check as before
4. hh_single_type_check --batch-mode writes to filename.out directly for each file instead of printing all of the errors.
5.  We then examine these .out files the same way we did before.

We can probably extend this to do the same thing for our other tests, but this certainly makes it a lot easier for typechecking.

Perf improvement:
Previous typechecker tests would take up to 200 seconds on my devserver. **Now, it takes 6 seconds**.

Reviewed By: vassilmladenov

Differential Revision: D12812688

fbshipit-source-id: 5ba6fdd5c9a12e109a8290856aecc5d7681319d7

hphp/hack/src/hh_single_type_check.ml

@@ -53,7 +53,6 @@ type mode =
   | Dump_symbol_info
   | Dump_inheritance
   | Errors
-  | AllErrors
   | Lint
   | Dump_deps
   | Identify_symbol of int * int
@@ -70,11 +69,13 @@ type mode =
   | Infer_return_types
   | Least_upper_bound
   | Linearization
+  | Batch_errors
 
 type options = {
   files : string list;
   mode : mode;
   no_builtins : bool;
+  all_errors : bool;
   tcopt : GlobalOptions.t;
 }
 
@@ -121,8 +122,21 @@ let die str =
   Out_channel.close oc;
   exit 2
 
-let print_error ?(indent=false) l =
-  Out_channel.output_string stderr (Errors.to_string ~indent (Errors.to_absolute l))
+
+let print_error ?(oc = stderr) ?(indent=false) l =
+  Out_channel.output_string oc (Errors.to_string ~indent (Errors.to_absolute l))
+
+let write_error_list errors oc =
+  (if errors <> []
+  then List.iter ~f:(print_error ~oc ~indent:true) errors
+  else Out_channel.output_string oc "No errors\n");
+  Out_channel.close oc
+
+let write_first_error errors oc =
+  (if errors <> []
+  then print_error ~oc (List.hd_exn errors)
+  else Out_channel.output_string oc "No errors\n");
+  Out_channel.close oc
 
 let print_error_list errors =
   if errors <> []
@@ -171,12 +185,13 @@ let parse_options () =
   let unsafe_rx = ref false in
   let disallow_stringish_magic = ref false in
   let unresolved_as_union = ref false in
+  let all_errors = ref false in
   let options = [
     "--ai",
       Arg.String (set_ai),
     " Run the abstract interpreter";
     "--all-errors",
-      Arg.Unit (set_mode AllErrors),
+      Arg.Set all_errors,
       " List all errors not just the first one";
     "--deregister-attributes",
       Arg.Set deregister_attributes,
@@ -343,7 +358,10 @@ let parse_options () =
         " Disallow using objects in contexts where strings are required.";
     "--unresolved-as-union",
         Arg.Set unresolved_as_union,
-        " Interpret unresolved in type inference only as union."
+        " Interpret unresolved in type inference only as union.";
+    "--batch-files",
+        Arg.Unit (set_mode Batch_errors),
+        " Typecheck each file passed in independently";
   ] in
   let options = Arg.align ~limit:25 options in
   Arg.parse options (fun fn -> fn_ref := fn::(!fn_ref)) usage;
@@ -390,6 +408,7 @@ let parse_options () =
   { files = fns;
     mode = !mode;
     no_builtins = !no_builtins;
+    all_errors = !all_errors;
     tcopt;
   }
 
@@ -744,7 +763,7 @@ let typecheck_tasts tasts tcopt (filename:Relative_path.t) =
   List.concat_map tasts ~f:typecheck_tast
 
 let handle_mode
-  mode filenames tcopt popt files_contents files_info parse_errors =
+  mode filenames tcopt popt files_contents files_info parse_errors all_errors =
   let expect_single_file () : Relative_path.t =
     match filenames with
     | [x] -> x
@@ -1025,23 +1044,39 @@ let handle_mode
   | Infer_return_types
   | Errors ->
       (* Don't typecheck builtins *)
-      let files_info = Relative_path.Map.fold builtins
+      let files_info = if all_errors then files_info else
+      Relative_path.Map.fold builtins
         ~f:begin fun k _ acc -> Relative_path.Map.remove acc k end
-        ~init:files_info
-      in
+        ~init:files_info in
       let errors = check_file tcopt parse_errors files_info in
       if mode = Infer_return_types
       then
         iter_over_files (fun filename ->
         Option.iter ~f:(infer_return tcopt filename)
           (Relative_path.Map.get files_info filename)
         );
-      print_first_error errors;
-      if errors <> [] then exit 2
-  | AllErrors ->
-      let errors = check_file tcopt parse_errors files_info in
-      print_error_list errors;
+      (if all_errors then
+        print_error_list errors
+      else
+        print_first_error errors);
       if errors <> [] then exit 2
+  | Batch_errors ->
+      (* For each file in our batch, run typechecking serially.
+        Reset the heaps every time in between. *)
+      (* This means builtins had errors, so lets just print those if we see them *)
+      if parse_errors <> [] then (print_error (List.hd_exn parse_errors); exit 2);
+      iter_over_files (fun filename ->
+        let oc = Out_channel.create ((Relative_path.to_absolute filename) ^ ".out") in
+        Typing_log.out_channel := oc;
+        ServerIdeUtils.make_local_changes ();
+        let files_contents = file_to_files filename in
+        let parse_errors, individual_file_info = parse_name_and_decl popt files_contents tcopt in
+        let errors = check_file tcopt (Errors.get_error_list parse_errors) individual_file_info in
+        (if all_errors then write_error_list errors oc
+        else write_first_error errors oc);
+        ServerIdeUtils.revert_local_changes ()
+      )
+
   | Decl_compare ->
     let filename = expect_single_file () in
     test_decl_compare filename popt files_contents tcopt files_info
@@ -1081,13 +1116,11 @@ let handle_mode
       )
     )
 
-
-
 (*****************************************************************************)
 (* Main entry point *)
 (*****************************************************************************)
 
-let decl_and_run_mode {files; mode; no_builtins; tcopt } popt =
+let decl_and_run_mode {files; mode; no_builtins; tcopt; all_errors } popt =
   if mode = Dump_deps then Typing_deps.debug_trace := true;
   Ident.track_names := true;
   let builtins = if no_builtins then Relative_path.Map.empty else builtins in
@@ -1102,11 +1135,13 @@ let decl_and_run_mode {files; mode; no_builtins; tcopt } popt =
     ~f:begin fun k src acc -> Relative_path.Map.add acc ~key:k ~data:src end
     ~init:files_contents
   in
-
+  (* Don't declare all the filenames in batch_errors mode *)
+  let to_decl = if mode = Batch_errors then builtins else files_contents_with_builtins in
   let errors, files_info =
-    parse_name_and_decl popt files_contents_with_builtins tcopt in
+    parse_name_and_decl popt to_decl tcopt in
+
   handle_mode mode files tcopt popt files_contents files_info
-    (Errors.get_error_list errors)
+    (Errors.get_error_list errors) all_errors
 
 let main_hack ({files; mode; tcopt; _} as opts) =
   (* TODO: We should have a per file config *)

hphp/hack/src/server/serverIdeUtils.mli

@@ -7,6 +7,9 @@
  *
  *)
 
+
+val make_local_changes: unit -> unit
+val revert_local_changes: unit -> unit
 (* When typechecking a content buffer in IDE mode,
 * this is the path that will be assigned to it *)
 val path: Relative_path.t

hphp/hack/src/typing/typing_log.ml

@@ -17,6 +17,7 @@ open Tty
 (*****************************************************************************)
 
 (* Eventual input to Tty.cprint *)
+let out_channel = ref stdout
 let logBuffer = ref []
 let indentLevel = ref 0
 let accumulatedLength = ref 0
@@ -25,7 +26,7 @@ let lnewline () =
   | [] -> ()
   | _ ->
     begin
-      cprint ~color_mode:Color_Auto
+      cprint ~out_channel:!out_channel ~color_mode:Color_Auto
         ((Normal White, String.make (2 * !indentLevel) ' ') ::
            List.rev ((Normal White, "\n") :: !logBuffer));
        accumulatedLength := 0;

hphp/hack/test/verify.py

@@ -15,6 +15,7 @@
 max_workers = 48
 verbose = False
 dump_on_failure = False
+batch_size = 100
 
 
 TestCase = namedtuple('TestCase', ['file_path', 'input', 'expected'])
@@ -39,10 +40,76 @@ def get_test_flags(path: str) -> List[str]:
         return shlex.split(file.read().strip())
 
 
+def run_batch_tests(test_cases: List[TestCase],
+                    program: str,
+                    default_expect_regex,
+                    get_flags: Callable[[str], List[str]],
+                    out_extension: str) -> List[Result]:
+    """
+    Run the program with batches of files and return a list of results.
+    """
+    # Each directory needs to be in a separate batch because flags are different
+    # for each directory.
+    # Compile a list of directories to test cases, and then
+    dirs_to_files : Dict[str, List[TestCase]] = {}
+    for case in test_cases:
+        test_dir = os.path.dirname(case.file_path)
+        dirs_to_files.setdefault(test_dir, []).append(case)
+
+    # run a list of test cases.
+    # The contract here is that the program will write to
+    # filename.out_extension for each file, and we read that
+    # for the output.
+    def run(test_cases : List[TestCase]):
+        if not test_cases:
+            assert False
+        first_test = test_cases[0]
+        test_dir = os.path.dirname(first_test.file_path)
+        flags = get_flags(test_dir)
+        test_flags = get_test_flags(first_test.file_path)
+        cmd = [program, "--batch-files"]
+        cmd += flags + test_flags
+        cmd += [os.path.basename(case.file_path) for case in test_cases]
+        if verbose:
+            print('Executing', ' '.join(cmd))
+        try:
+            subprocess.call(
+                cmd, stderr=subprocess.STDOUT, cwd=test_dir,
+                universal_newlines=True)
+        except subprocess.CalledProcessError as e:
+            # we don't care about nonzero exit codes... for instance, type
+            # errors cause hh_single_type_check to produce them
+            pass
+        results = []
+        for case in test_cases:
+            with open(case.file_path + out_extension, "r") as f:
+                output : str = f.read()
+                result = check_result(case, default_expect_regex, output)
+                results.append(result)
+        return results
+    # Create a list of batched cases.
+    all_batched_cases : List[List[TestCase]] = []
+
+    # For each directory, we split all the test cases
+    # into chunks of batch_size. Then each of these lists
+    # is a separate job for each thread in the threadpool.
+    for cases in dirs_to_files.values():
+        batched_cases : List[List[TestCase]] = \
+            [cases[i:i + batch_size] for i in range(0, len(case), batch_size)]
+        all_batched_cases += batched_cases
+
+    executor = ThreadPoolExecutor(max_workers=max_workers)
+    futures = [executor.submit(run, test_batch) for test_batch in all_batched_cases]
+
+    results = [future.result() for future in futures]
+    # Flatten the list
+    return [item for sublist in results for item in sublist]
+
 def run_test_program(test_cases: List[TestCase],
                      program: str,
                      default_expect_regex,
                      get_flags: Callable[[str], List[str]]) -> List[Result]:
+
     """
     Run the program and return a list of results.
     """
@@ -213,17 +280,22 @@ def run_tests(files: List[str],
               use_stdin: str,
               program: str,
               default_expect_regex,
+              batch_mode: str,
               get_flags: Callable[[str], List[str]]) -> List[Result]:
+
     # for each file, create a test case
     test_cases = [
         TestCase(
             file_path=file,
             expected=get_content(file, expected_extension),
             input=get_content(file) if use_stdin else None)
         for file in files]
-
-    results = run_test_program(test_cases, program, default_expect_regex,
-        get_flags)
+    if batch_mode:
+        results = run_batch_tests(test_cases, program, default_expect_regex,
+            get_flags, out_extension)
+    else:
+        results = run_test_program(test_cases, program, default_expect_regex,
+            get_flags)
 
     failures = [result for result in results if result.is_failure]
 
@@ -322,6 +394,8 @@ def abspath(path: str) -> str:
     parser.add_argument('--flags', nargs=argparse.REMAINDER)
     parser.add_argument('--stdin', action='store_true',
                         help='Pass test input file via stdin')
+    parser.add_argument('--batch', action='store_true',
+                        help='Run tests in batches to the test program')
     parser.epilog = "Unless --flags is passed as an argument, "\
                     "%s looks for a file named HH_FLAGS in the same directory" \
                     " as the test files it is executing. If found, the " \
@@ -357,6 +431,7 @@ def abspath(path: str) -> str:
         args.stdin,
         args.program,
         args.default_expect_regex,
+        args.batch,
         get_flags)
 
     # Doesn't make sense to check failures for idempotence