Repository navigation
Conversation
JonoPrest
added a commit
that referenced
this pull request
Nov 3, 2026
The entry names #9194. Signed-off-by: Jono Prest <[email protected]>
Contributor
Author
|
@codex review |
Changelog💅 Polish
Semantic Diff2 commits · Full page Copy a dependency's values when they are read6d73a8e · JonoPrest · Nov 03, 2026, 11:26 UTC
For the components of a .cmi signature as it was read, values are now table cells that make the copy on first read. Type ids are allocated in order and end up in .cmt files and in which nodes the backtracking trail logs, so the copies keep their ids: once the process has built the components of the same .cmi contents (same CRC) eagerly, it knows how many type ids each value takes, reserves them when the components are built and makes the copy with exactly those ids when the value is read. A copy that takes a different number is made again with fresh ids, like the deferred signature copies, and counted per domain for the tests. One-shot bsc and the first use of an interface in a build still build eagerly. Types, constructors, labels, extension constructors, module types, submodules and components of anything else (aliases, functor applications, local modules) are built as before. The tables of an open take the cells too: an open keeps a function giving the binding of each component, and the bindings Env.copy_types replaces through an open in a table checked before the components. Review
Allocate a value's copy closure only when the copy is deferredb9df7dd · JonoPrest · Nov 09, 2026, 14:34 UTC
Measured with scripts/compilerBenchmarks.js against the previous commit: allocated words -0.15% in total, -0.25% in the project builds (where components are built again in one process and their counts are known), -0.04% for the files compiled one by one. On a 552 module React app (clean build, one domain) the build allocates 0.2% fewer words. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## jono/defer-signature-copy #9194 +/- ##
=============================================================
+ Coverage 81.95% 82.02% +0.07%
=============================================================
Files 457 458 +1
Lines 65620 65964 +344
=============================================================
+ Hits 53780 54109 +329
- Misses 11840 11855 +15
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
JonoPrest
marked this pull request as ready for review
November 3, 2026 11:56
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 3, 2026 19:51
8810538 to
67eb3bc
Compare
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 3, 2026 21:51
67eb3bc to
94bd206
Compare
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 4, 2026 06:15
94bd206 to
c342131
Compare
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 4, 2026 17:14
c342131 to
87d814f
Compare
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 9, 2026 07:18
87d814f to
f8f3ba1
Compare
🎉 Compiler benchmarksAllocated wordsIn total: 680,168,100 → 675,146,415 (-0.74%). 🎉 Improvement: 2 benchmarks allocate more than 1% less.
221 more changes
InstructionsIn total: 9,664,627,709 → 9,667,238,667 (+0.03%).
Output sizeIn total: 25,570,910 → 25,572,544 (+0.01%). None of the 13 benchmarks changed by 0.1% or more. Pull request b9df7dd compared with base 462c00f in this run. |
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 9, 2026 07:34
f8f3ba1 to
1ddb5e1
Compare
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 9, 2026 12:01
1ddb5e1 to
31cc3ad
Compare
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 9, 2026 12:41
31cc3ad to
787e4e0
Compare
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 9, 2026 13:05
787e4e0 to
3f453f9
Compare
4 tasks done
Why:
Forcing a dependency's components copied every value of its interface
with the module path substitution, although a dependent reads only a
few of them: on a 552-module app, about 5 % of the values of .cmi
components are ever read.
For the components of a .cmi signature as it was read, values are now
table cells that make the copy on first read. Type ids are allocated in
order and end up in .cmt files and in which nodes the backtracking trail
logs, so the copies keep their ids: once the process has built the
components of the same .cmi contents (same CRC) eagerly, it knows how many
type ids each value takes, reserves them when the components are built and
makes the copy with exactly those ids when the value is read. A copy that
takes a different number is made again with fresh ids, like the deferred
signature copies, and counted per domain for the tests. One-shot bsc and
the first use of an interface in a build still build eagerly. Types,
constructors, labels, extension constructors, module types, submodules and
components of anything else (aliases, functor applications, local modules)
are built as before.
The tables of an open take the cells too: an open keeps a function giving
the binding of each component, and the bindings Env.copy_types replaces
through an open in a table checked before the components.
Review: Can a deferred copy differ from the eager one apart from when it is
made? The copied values are the unmarshalled .cmi signature, which the type
checker never sees (it only gets copies), and the copy takes no stamps; the
type ids are the eager ones, so every later stamp and id, and every
operation on the copied nodes, is the same. The new unit tests compare
marshalled results of eager and deferred components over every runtime
interface, and whole requests (outputs, .cmt and error messages) using
functors, aliases, include, module type of, recursive modules, shadowing
and first-class modules, also on several domains at once.
Review: Are the cells shared between requests or domains? No: components
belong to the persistent structures of one request; only the count table is
shared, behind a mutex, and its arrays are not changed once recorded.
Signed-off-by: Jono Prest <[email protected]>
Why:
`components_of_module_maker` built a closure for the copy of every
value it added, also when it then copied the value at once (the first
time the process builds a .cmi's components, and for every module
that is not part of a .cmi), and checked recorded counts against the
signature by converting the count array to a list.
`make_cell` now takes the substitution and the declaration and builds a
closure only for a deferred copy, and the counts are checked by walking
the signature with an index. Which values are deferred, the type ids
they take and the components built are unchanged.
Measured with scripts/compilerBenchmarks.js against the previous commit:
allocated words -0.15% in total, -0.25% in the project builds (where
components are built again in one process and their counts are known),
-0.04% for the files compiled one by one. On a 552 module React app
(clean build, one domain) the build allocates 0.2% fewer words.
Signed-off-by: Jono Prest <[email protected]>
JonoPrest
force-pushed
the
jono/defer-dependency-value-copies
branch
from
November 9, 2026 16:43
03a22d2 to
b9df7dd
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #9192.
When a module first uses a dependency,
Envbuilds that dependency's components and copies every value of its interface with the module path substitution. Dependents read only a few of them: on a 552 module React app about 5% of those values are ever read.For the components of a
.cmisignature, values are now table cells that make the copy on first read. This uses the same approach as #9192 to keep output identical:.cmi,.cmj, JS and.cmt, stays the same.bscand the first use of an interface in a build are unchanged.opentables take the cells too: an open keeps a function giving each component's binding, andEnv.copy_typesoverrides go in a small table checked first.The gain is real but modest, and this adds a fair amount of code to
env.ml(about 300 lines). Paired benchmark, 20 interleaved clean builds per project, against #9192, user CPU with bootstrap 95% CI (for the full version, deferring types and labels too; this values only version measured the same head to head, with about 30% less code):Wall time at default domains is within noise on two of the three projects. Allocation drops about 2% on the React app. So this is a few percent of CPU, and only in the long running rewatch process. Happy to drop it if that isn't worth the extra machinery in
env.ml.Testing:
ounit_deferred_components_tests.ml: eager vs deferred components of every runtime interface compared as marshalled results, unread values still reserving their ids, the fallback for a wrong count, a copy that raises,Env.copy_typesthrough an open, and whole compiler requests (outputs,.cmtand error messages with functors, aliases, include,module type of, recursive modules, shadowing, first class modules) eager vs deferred, also on 4 domains at once..cmi,.cmj,.ast,.cmtand.cmtiare byte identical to master across three production codebases and their dependencies, at 1 and the default number of domains.make testandmake test-rewatchpass.