Repository navigation
Commit ac73578
Read build files once and publish outputs from memory in OCaml rewatch (#9170)
* Read each build file once per OCaml rewatch build
Why:
Every in-process compile read the interfaces it uses from disk again,
above all the runtime's, and unmarshalled them again, and finding one
took a failed `open` in each load-path directory before the one that
has it. On macOS an `open` costs about 15 µs and gets slower when
several threads call it at once, so a clean build of a 2000-module
project spent a third of its CPU time in `open`.
The compiler now reads and writes build files through `Build_file_store`.
Rewatch enables it for the duration of each build: a file is read from
disk at most once, a file written through the store is kept for later
reads, and each domain keeps the interfaces it has unmarshalled until the
file is written again. `bsc` leaves the store disabled and reads the disk
as before. Writers render their output in memory first, so the bytes
written are unchanged: every artifact of the fixtures below, `.cmt` files
included, is byte-identical to master's.
IO modified (file system calls of a clean build, 2000 modules, 8 domains):
Before (text):
open .cmi 70856
stat .cmj 30856
open .cmj 14856
open .res 10000
open .ast 8000
all calls 211523
After (text):
open .cmi 5454
stat .cmj 1731
open .cmj 5722
open .res 8000
open .ast 6000
all calls 101462
Build modified (wall time, median of 5, Apple M4 Pro):
Before (text):
synthetic 2000 modules, clean 2.42 s
tests/tests, clean 0.71 s
After (text):
synthetic 2000 modules, clean 2.11 s
tests/tests, clean 0.67 s
Review: Can a request read a stale file from the store? Within a build a
file is read only after the request that writes it has finished, because
rewatch orders requests by their dependencies, and writes go through the
store. Requests spell paths relative to their working directory, such as
`lib/bs/../ocaml/A.cmi`, so keys remove `.` and `..` lexically. The store
starts every build empty, so it sees changes made between builds.
Review: Is it safe to reuse an unmarshalled interface in later requests?
Only on the domain that read it: copying a signature marks its types in
place until the copy ends, which is why domains don't share them. A request
that fails drops the domain's interfaces, in case it stopped with a type
still marked, and every request resets the abbreviations it memoized,
including those in cached interfaces.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Read AST headers once, in parallel, in OCaml rewatch
Why:
Before anything compiles, a build reads the header of every AST twice
on the main domain: the published copy in lib/ocaml, to know its
source and dependencies, and again the one in lib/bs, to build the
module graph. For a 2000-module project that was two thirds of a build
after a one-file edit.
The module graph now takes the dependencies of a module that wasn't parsed
in this build from its published AST, which is a copy of the one in
lib/bs, and those of a module parsed in this build from memory. The
published ASTs are inventoried on all worker domains, and a header is read
with one small read instead of through a channel that buffers 64 KiB of
the AST.
Build modified (wall time after a one-file edit, median of 5, Apple M4 Pro):
Before (text): synthetic 2000-module project
no-op build 0.139 s
edit a leaf module 0.155 s
After (text): synthetic 2000-module project
no-op build 0.089 s
edit a leaf module 0.104 s
Review: When can the published AST's dependencies differ from the one in
lib/bs? The published AST is a byte copy made when the module is parsed.
A module whose source is newer than its published AST, or whose published
AST is missing, is parsed again, and its dependencies are then read from
the new AST in memory.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Publish outputs from memory, on the worker that made them, in OCaml rewatch
Why:
Publishing copies each output of the compiler into lib/ocaml, and each
source into lib/bs and lib/ocaml, by reading back the files just
written. After parsing, the main domain alone also copied every AST
and source, one after another: in a clean build of a 2000-module
project all workers sat idle for half a second.
Copies now take their contents from `Build_file_store`, where the compiler
left what it wrote, and keep the published interfaces and `.cmj` files
for dependents to read; ASTs, typed trees and JavaScript leave the store
once published. Each parse publishes its AST and source on the worker
that ran it. The source copy that compiling publishes again is written
only if it differs from the one the parse published. Every rewatch-side
write or removal of a file updates the store.
IO modified (file system calls of a clean build, 2000 modules, 8 domains):
Before (text):
all calls 99063
After (text):
all calls 69125
Build modified (wall time, median of 5, Apple M4 Pro):
Before (text):
synthetic 2000 modules, clean 2.07 s
After (text):
synthetic 2000 modules, clean 1.80 s
Review: Is a file a hook or PPX reads still on disk when it runs? Yes:
writes through the store reach the disk before `write` returns; the store
only spares reading them back.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Test the build file store's failure paths
Codecov flagged the store's failure handling as untested. The new cases
check that an interface cut short reads as corrupted from memory as it
does from disk, that a failed write keeps nothing for its path, and that
a failed request makes its domain read its interfaces again while a
successful one keeps them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Correct the clean-build figure in the build file store's changelog entry
The entry said clean builds became about a quarter faster, as measured
with the disk nearly full. Measured again with free space, a clean build
of the synthetic 2000-module project takes 2.21 s on master and 1.47 s
with this branch, about a third less.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Say in the OCaml rewatch guide that workers publish their outputs
The guide still said the main domain publishes every artifact, which
stopped being true when workers began publishing what they produce from
memory.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Read an AST's header with one parser, in the compiler and OCaml rewatch
Why:
The header of a binary AST was parsed in three places: in
`Binary_ast.read_ast_exn`, and twice in OCaml rewatch's `Ast_header`,
once to find where the header ends in the first block read and once
to read it. `read_ast_exn` also raised `Invalid_argument`, not
`End_of_file`, for a header with a negative or impossible dependency
length.
`Binary_ast.read_header` finds where the dependency block and the source
path end in the first bytes of an AST, or says they don't hold the whole
header. `read_ast_exn` raises `End_of_file` for a file without one, and
rewatch reads one block of a published AST, and all of it only when the
header may be longer.
Review: Does rewatch read any header differently? An AST with no newline
after its source path now reads as damaged, so its module parses again;
before, rewatch took the path to run to the end of the file. `write_ast`
always writes the newline, so only a file cut short ends this way.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Publish each parse from its task's result hook, in OCaml rewatch
Why:
Parses were published through an `on_success` hook added to
`Compiler_process.task` and `run_requests`, which looked each parse up
by its index, though `Process.map_result` already runs a function on
the worker when a task finishes. Compile assets also regrouped the
inventory by directory through a hashtable, though `Parallel.map`
returns it in the order of the directories.
`run_parses` maps each parse task's result to publish it, and `task` and
`run_requests` are as they were before the build file store.
`Compile_assets.create` takes each directory's entries in turn. The store
says why its `enabled` and `generation` fields are read without its lock.
Review: Is a parse still published on its worker, and does a failed
publication still fail the parse? `Process.run_concurrent_task` applies
the result hook on the worker, inside the handler that turns an exception
into a failed task.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Take a parsed module's dependencies from its new AST, in OCaml rewatch
Why:
The module graph reads a module's dependencies from the header of its
published AST, which the build read before parsing. It told a module
parsed in this build by its AST being held in the build file store, so
a store that no longer held the AST, or no store, would have given the
module the dependencies of its previous source, and the build would
have skipped recompiles.
`Module_graph.initialize` takes the paths that this build parsed
successfully, which `Build_preparation` already knows, and reads their
dependencies from their new AST in `lib/bs`. The store's copy of that AST
only saves the disk read.
Review: Does any other caller rely on the store to tell a fresh parse?
The incremental path in `Build` reads every changed module's AST from
`lib/bs` without the published headers, as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Bound each domain's interface cache, in the compiler
Why:
Every compiler worker domain kept each interface it decoded for the
rest of the build. A decoded interface takes a few times the size of
its file, so a project with large interfaces could hold several copies
of all of them, one per domain.
A domain caches interfaces whose contents add up to at most 16 MB, and
starts over when the next one would exceed that. The runtime's interfaces
take 2 MB. An interface rewritten during the build replaces its entry and
its size.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Raise End_of_file for a truncated AST or .cmj, in the compiler
Why:
Reading from the build file store, an AST or `.cmj` cut short inside
its marshalled value raised `Invalid_argument` from
`Marshal.from_string`, though the AST reader is documented to raise
`End_of_file` for a file a killed build cut short. A channel's
`input_value` raised `Failure` there.
`Ext_io.unmarshal` checks that the marshalled value ends within the
contents, and raises `End_of_file` when it doesn't. The AST and `.cmj`
readers use it, and read their file with `Ext_io.load_file` instead of
their own copy of it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Keep the build file store in step with removed files, in the compiler and OCaml rewatch
Why:
`Misc.remove_file`, which removes stale source maps and the `.cmi` of
a failed save, deleted files without telling the build file store, so
a later read in the same build would have returned the removed
contents. Nothing reads those paths again today. An implementation
compiled with `-bs-read-cmi` also read its interface's `.cmi` in
`lib/bs` back into the store, which then kept it for the rest of the
build. And when `Parallel.map` failed to start a domain, it raised
without joining the domains it had started.
`Misc.remove_file` forgets the path it removes, and publishing an
implementation that has an interface evicts the `.cmi` that its compile
read. `Parallel.map` shares the items among the domains that started. The
store's interface no longer suggests `keep:false` for `.cmt` files, which
rewatch copies out of the store.
New tests cover a removed file, a file that exists but can't be read,
several domains using the store at once, more spellings of one path, and
AST headers that end at the edge of the first read.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Test which files publishing keeps in the build file store, in OCaml rewatch
Why:
Publishing a module keeps its interface and `.cmj` in the store for
its dependents, and evicts the rest so that a clean build's memory
stays bounded. No test checked which files it kept, so an eviction
that was dropped would only have shown as higher peak memory.
The new test publishes a module and checks that the published `.cmi` and
`.cmj` stay in the store, while the typed tree, the AST, the sources, the
JavaScript output and every copy leave it. It also publishes an
implementation that has an interface, whose compile reads its `.cmi` in
`lib/bs` again, and checks that this `.cmi` leaves the store too. Each
check fails without the eviction it covers.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Publish parses that print warnings after their cleanup, in OCaml rewatch
Why:
A worker published each successful parse into `lib/ocaml` as soon as
it finished, including a parse that printed warnings. The cleanup
that removes such an AST, so that the next build parses the source
again and repeats its warnings, is only registered once all the
package's parses are done. A build interrupted between the two left
the AST published and newer than its source, so the next build
skipped the parse and never reported warnings such as the deprecated
`(. ...)` syntax.
The worker now publishes only parses that printed nothing.
`Package_parse` publishes the others itself, after it registers their
cleanup, as it did for every parse before the worker took over
publication.
A new test parses one clean source and one that warns, and checks that
only the clean one is published.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
* Merge the two OCaml rewatch build I/O changelog entries into one
The branch added one entry for the build file store and publication and
another for the AST header scan. Each said it made builds about a third
faster, without saying which builds, so they read as two similar claims.
One entry now describes the three steps together, with the figures the
pull request measured on the synthetic 2000-module project: a clean
build takes 2.21 s on master and 1.47 s with the branch, a no-op build
0.154 s and 0.089 s, and a build after editing a leaf module 0.174 s and
0.100 s.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
---------
Signed-off-by: Paul Tsnobiladzé <2479216+tsnobip@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>1 parent c10223e commit ac73578
38 files changed
Lines changed: 1609 additions & 293 deletions
File tree
- changes
- compiler
- bsc
- core
- depends
- ext
- ml
- platform/native
- rewatch-ocaml
- tests
- ounit_tests
- rewatch_ounit_tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
569 | 569 | | |
570 | 570 | | |
571 | 571 | | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
572 | 587 | | |
573 | 588 | | |
574 | 589 | | |
575 | 590 | | |
576 | 591 | | |
577 | | - | |
| 592 | + | |
| 593 | + | |
578 | 594 | | |
579 | 595 | | |
580 | 596 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
25 | 92 | | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | 93 | | |
30 | 94 | | |
31 | 95 | | |
32 | | - | |
33 | | - | |
34 | | - | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
35 | 99 | | |
36 | | - | |
37 | | - | |
38 | | - | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
39 | 103 | | |
40 | | - | |
41 | | - | |
42 | | - | |
43 | | - | |
| 104 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
27 | 28 | | |
28 | 29 | | |
29 | 30 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
73 | | - | |
74 | 72 | | |
75 | | - | |
76 | | - | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
| 73 | + | |
81 | 74 | | |
82 | 75 | | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
90 | | - | |
91 | | - | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
92 | 81 | | |
93 | 82 | | |
94 | 83 | | |
| |||
98 | 87 | | |
99 | 88 | | |
100 | 89 | | |
101 | | - | |
102 | | - | |
103 | | - | |
104 | | - | |
105 | | - | |
106 | | - | |
107 | | - | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
108 | 93 | | |
109 | 94 | | |
110 | 95 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
453 | 453 | | |
454 | 454 | | |
455 | 455 | | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
456 | 472 | | |
457 | | - | |
| 473 | + | |
458 | 474 | | |
459 | 475 | | |
460 | 476 | | |
461 | 477 | | |
462 | 478 | | |
463 | 479 | | |
464 | 480 | | |
465 | | - | |
| 481 | + | |
466 | 482 | | |
467 | 483 | | |
468 | 484 | | |
469 | | - | |
| 485 | + | |
470 | 486 | | |
471 | 487 | | |
472 | 488 | | |
473 | 489 | | |
474 | 490 | | |
475 | | - | |
| 491 | + | |
476 | 492 | | |
477 | 493 | | |
478 | 494 | | |
| |||
490 | 506 | | |
491 | 507 | | |
492 | 508 | | |
493 | | - | |
| 509 | + | |
| 510 | + | |
494 | 511 | | |
495 | 512 | | |
496 | 513 | | |
| |||
505 | 522 | | |
506 | 523 | | |
507 | 524 | | |
508 | | - | |
509 | | - | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
510 | 528 | | |
511 | | - | |
512 | | - | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
513 | 532 | | |
514 | 533 | | |
515 | 534 | | |
516 | | - | |
517 | | - | |
518 | | - | |
519 | | - | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
520 | 538 | | |
521 | 539 | | |
522 | 540 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
31 | | - | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | | - | |
36 | | - | |
37 | | - | |
38 | | - | |
39 | | - | |
40 | | - | |
41 | | - | |
42 | 31 | | |
43 | 32 | | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
44 | 67 | | |
45 | 68 | | |
46 | 69 | | |
| |||
57 | 80 | | |
58 | 81 | | |
59 | 82 | | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
| 29 | + | |
28 | 30 | | |
29 | 31 | | |
30 | 32 | | |
| |||
33 | 35 | | |
34 | 36 | | |
35 | 37 | | |
36 | | - | |
37 | | - | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
0 commit comments