Skip to content

Commit 8a61aaa

Browse files
committed
Start dependents once a module's interface is final
Why: On builds limited by a long dependency chain most job slots sit idle on the critical path: a 136 module library gets only 1.6x from 7 domains. When a slot would otherwise idle, a module can now start as soon as each unfinished dependency's interface is final: an implementation without .resi has computed the .cmi it is about to write, or an interface file has compiled. The early request reads those .cmi files from memory and waits at a gate before writing a file, running a command or returning its result. The scheduler adopts it at the point it would have started it (dependencies complete, module still dirty), so its code generation sees the dependencies' .cmj files, and artifacts, diagnostics and compiler.log are unchanged. Otherwise it is abandoned without effects, together with anything that started on top of it. Redirected paths are matched as the build file store keys them. The directories in which the build publishes interfaces are spelled from canonical package roots, both in the redirected paths and on the load path, so a lookup there that matches no redirected path names another file. A lookup of a redirected file name in any other directory, which may be an alias from user flags (with -nostdlib those come first), or one that differs only in case, waits for adoption instead. Whether an announced interface changes the published one is read from disk, so that an abandoned request never fills the store. Modules with js-post-build commands or lowercase .cmi names don't release their dependents early, and with one domain nothing changes. Review: Can a dependent see an interface that differs from master's? It reads the exact contents the dependency then writes, and it is only adopted after the dependency completed. If the dependency fails later, for example in code generation, the dependent is abandoned before it wrote, ran or reported anything. Signed-off-by: Jono Prest <jjprest@gmail.com>
1 parent 4f5effc commit 8a61aaa

21 files changed

Lines changed: 1972 additions & 81 deletions
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Build: start type checking a module as soon as the interfaces of its dependencies are final, instead of waiting for their JavaScript generation, which shortens builds limited by long dependency chains.

‎rewatch-ocaml/README.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,21 @@ going to the process's stdout and stderr. The compiler build identity is
7878
derived from the compiler sources, so compiler changes clean the build while
7979
rewatch-only changes don't.
8080

81+
When no ready module can take a free job slot, a module can start before its
82+
dependencies have finished: as soon as each unfinished dependency's interface
83+
is final. That is when the implementation of a module without an interface
84+
file has computed the `.cmi` it is about to write, or when an interface file
85+
has compiled. The early request reads those not yet published `.cmi` files
86+
from memory. It keeps its job slot while it waits, before it writes any file,
87+
runs any command or reports any result, until the scheduler would have
88+
started it anyway: its dependencies have completed and it still needs
89+
compiling. Otherwise it is abandoned without effects, as are early requests
90+
that depend on it. Code generation therefore always sees the `.cmj` of
91+
completed dependencies, and artifacts, diagnostics and `compiler.log` are
92+
those of a build without pipelining. A module with `js-post-build` commands
93+
does not release its dependents early. `REWATCH_SPECULATION_LOG` names a file
94+
to which tests append each early start, adoption and abandonment.
95+
8196
External PPXs, `js-post-build` hooks, and `--after-build` remain subprocesses
8297
under the process owners described below.
8398

‎rewatch-ocaml/compile_assets.ml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ let create directories =
128128
state_entries);
129129
state
130130

131+
let directories state =
132+
Hashtbl.fold
133+
(fun directory _ directories -> directory :: directories)
134+
state.files_by_directory []
135+
131136
let files state directory =
132137
Hashtbl.find_opt state.files_by_directory directory
133138
|> Option.value ~default:[]

‎rewatch-ocaml/compile_assets.mli‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ type t
88

99
val is_managed_basename : string -> bool
1010
val create : string list -> t
11+
12+
val directories : t -> string list
13+
(** The directories indexed, in which the build publishes interfaces. *)
14+
1115
val files : t -> string -> string list
1216
val ast_sources : t -> string -> ast_source list
1317
val ast_dependencies : t -> string -> string list

‎rewatch-ocaml/compiler_process.ml‎

Lines changed: 117 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,54 @@ let exit_code = function
4949
| Unix.WEXITED code -> code
5050
| Unix.WSIGNALED signal | Unix.WSTOPPED signal -> 128 + signal
5151

52-
let run_in_process ?poll (request : request) =
53-
log_compiler_request request;
52+
(* Paths are matched as the build file store keys them. The directories in
53+
which the build publishes interfaces are spelled from canonical package
54+
roots, both in a redirected path and on the load path, so a lookup there
55+
that matches no redirected path names another file. Any other directory,
56+
such as one from user flags, may be an alias of one of them, and a file
57+
that differs only in case may be the same one: a lookup there of a
58+
redirected file name waits, as the file may be stale until then. *)
59+
let cmi_redirect ~cwd ~cmi_directories ~wait_for_adoption redirects =
60+
match redirects with
61+
| [] -> fun _ -> None
62+
| redirects -> (
63+
let key path =
64+
Build_file_store.key
65+
(if Filename.is_relative path then Filename.concat cwd path else path)
66+
in
67+
let table length = Hashtbl.create length in
68+
let contents_by_key = table (List.length redirects) in
69+
let folded_keys = table (List.length redirects) in
70+
let folded_names = table (List.length redirects) in
71+
List.iter
72+
(fun (published, contents) ->
73+
let key = key published in
74+
Hashtbl.replace contents_by_key key contents;
75+
Hashtbl.replace folded_keys (String.lowercase_ascii key) ();
76+
Hashtbl.replace folded_names
77+
(String.lowercase_ascii (Filename.basename key))
78+
())
79+
redirects;
80+
let directories = table (List.length cmi_directories) in
81+
List.iter
82+
(fun directory -> Hashtbl.replace directories (key directory) ())
83+
cmi_directories;
84+
fun path ->
85+
let key = key path in
86+
match Hashtbl.find_opt contents_by_key key with
87+
| Some _ as contents -> contents
88+
| None ->
89+
let folded = String.lowercase_ascii key in
90+
if
91+
Hashtbl.mem folded_names (Filename.basename folded)
92+
&& (Hashtbl.mem folded_keys folded
93+
|| not (Hashtbl.mem directories (Filename.dirname key)))
94+
then wait_for_adoption ();
95+
None)
96+
97+
let run_in_process ?poll ?(log = true)
98+
?(hooks = Compiler_request_state.no_hooks) (request : request) =
99+
if log then log_compiler_request request;
54100
match List.rev request.args with
55101
| [] ->
56102
{
@@ -60,7 +106,7 @@ let run_in_process ?poll (request : request) =
60106
}
61107
| input :: reversed_argv -> (
62108
match
63-
Rescript_compiler_driver.run_request ~cwd:request.cwd
109+
Rescript_compiler_driver.run_request_with_hooks ~hooks ~cwd:request.cwd
64110
~argv:(List.rev reversed_argv) ~input
65111
~run_external:
66112
(Some
@@ -95,15 +141,77 @@ let run ?poll request =
95141
Option.iter (fun poll -> poll ()) poll;
96142
result
97143

98-
let task request =
144+
let task ?(hooks = Compiler_scheduler.no_compile_hooks) request =
99145
let cancelled = Atomic.make false in
146+
let gate = hooks.Compiler_scheduler.gate in
147+
(* A speculation logs its call once adopted, on its worker as before, so
148+
that the test-only REWATCH_COMPILER_CALL_LOG lists only adopted calls. *)
149+
(* A logging failure fails every later gate pass too, the final one outside
150+
the compiler included, so nothing is written after it and the task fails
151+
as when a request logs at its start. The task runs on one domain. *)
152+
let logged = ref (if Option.is_none gate then Ok () else Error None) in
153+
let wait_for_adoption () =
154+
Option.iter Speculation_gate.wait gate;
155+
match !logged with
156+
| Ok () -> ()
157+
| Error (Some error) -> raise error
158+
| Error None -> (
159+
match log_compiler_request request with
160+
| () -> logged := Ok ()
161+
| exception error ->
162+
logged := Error (Some error);
163+
raise error)
164+
in
165+
let redirect =
166+
cmi_redirect ~cwd:request.cwd ~cmi_directories:hooks.cmi_directories
167+
~wait_for_adoption hooks.cmi_redirects
168+
in
169+
let compiler_hooks =
170+
Compiler_request_state.
171+
{
172+
before_effect = wait_for_adoption;
173+
interface_computed =
174+
(match hooks.on_interface_computed with
175+
| None -> fun _ _ -> ()
176+
| Some computed ->
177+
fun path bytes ->
178+
computed
179+
(if Filename.is_relative path then
180+
Filename.concat request.cwd path
181+
else path)
182+
bytes);
183+
redirect_cmi =
184+
(match gate with
185+
| None -> redirect
186+
| Some gate ->
187+
fun path ->
188+
(* Interfaces load throughout type checking: an abandoned
189+
speculation stops at the next one. *)
190+
if Speculation_gate.is_abandoned gate then
191+
raise Speculation_gate.Abandoned;
192+
redirect path);
193+
}
194+
in
100195
Process.concurrent_task
101-
~cancel:(fun () -> Atomic.set cancelled true)
196+
~cancel:(fun () ->
197+
Atomic.set cancelled true;
198+
Option.iter Speculation_gate.cancel gate)
102199
(fun () ->
103-
run_in_process
104-
~poll:(fun () ->
105-
if Atomic.get cancelled then raise (Process.Interrupted 15))
106-
request)
200+
(* An abandoned speculation that has not started yet does no work. *)
201+
Option.iter
202+
(fun gate ->
203+
if Speculation_gate.is_abandoned gate then
204+
raise Speculation_gate.Abandoned)
205+
gate;
206+
let result =
207+
run_in_process ~log:(Option.is_none gate) ~hooks:compiler_hooks
208+
~poll:(fun () ->
209+
if Atomic.get cancelled then raise (Process.Interrupted 15))
210+
request
211+
in
212+
(* A speculation's result, and its publication, wait for adoption. *)
213+
wait_for_adoption ();
214+
result)
107215

108216
let run_tasks ?poll ?on_complete tasks =
109217
Process.run_tasks

‎rewatch-ocaml/compiler_process.mli‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,22 @@ val run_parses :
2727
successful parse that printed nothing with {!publish_parse} on the worker
2828
that parsed it. The caller publishes the parses that printed warnings. *)
2929

30-
val task : request -> Process.task
30+
val task : ?hooks:Compiler_scheduler.compile_hooks -> request -> Process.task
31+
32+
val cmi_redirect :
33+
cwd:string ->
34+
cmi_directories:string list ->
35+
wait_for_adoption:(unit -> unit) ->
36+
(string * string) list ->
37+
string ->
38+
string option
39+
(** [cmi_redirect ~cwd ~cmi_directories ~wait_for_adoption redirects] maps a
40+
resolved .cmi candidate path to the contents redirected for it, matching
41+
paths as {!Build_file_store} does. [cmi_directories] are the directories in
42+
which the build publishes interfaces. A candidate that may name a
43+
redirected file under another spelling, because it differs from one only
44+
in case or lies in another directory under the same name, calls
45+
[wait_for_adoption] and is then read as usual. *)
3146

3247
val namespace_task :
3348
runtime:string ->

0 commit comments

Comments
 (0)