Skip to content

Copy a dependency's values only when they are read - #9194

Open
JonoPrest wants to merge 1 commit into
jono/defer-signature-copyfrom
jono/defer-dependency-value-copies
Open

JonoPrest wants to merge 1 commit into
jono/defer-signature-copyfrom
jono/defer-dependency-value-copies

Conversation

@JonoPrest

Copy link
Copy Markdown
Contributor

Stacked on #9192.

When a module first uses a dependency, Env builds 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 .cmi signature, values are now table cells that make the copy on first read. This uses the same approach as #9192 to keep output identical:

  • The first time the process builds the components of an interface (same CRC), it builds them eagerly and records how many type ids each value takes.
  • After that, building the components reserves those ids for each value, and the copy is made with exactly those ids when the value is read. Every stamp and type id, and so .cmi, .cmj, JS and .cmt, stays the same.
  • A copy that takes a different number of ids is made again with fresh ids. This doesn't happen on any project, and the tests check for it.
  • Types, constructors, labels, module types, submodules, and components of aliases, functor applications and local modules are built as before. One shot bsc and the first use of an interface in a build are unchanged.

open tables take the cells too: an open keeps a function giving each component's binding, and Env.copy_types overrides 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):

project 1 domain default domains
React app, 552 modules 0.967 [0.958, 0.978] 0.974 [0.952, 0.988]
backend service, 391 modules 0.973 [0.955, 0.984] 0.968 [0.936, 0.995]
library 0.952 [0.922, 0.991] 0.945 [0.923, 1.002]

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:

  • New 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_types through an open, and whole compiler requests (outputs, .cmt and error messages with functors, aliases, include, module type of, recursive modules, shadowing, first class modules) eager vs deferred, also on 4 domains at once.
  • JS, .cmi, .cmj, .ast, .cmt and .cmti are byte identical to master across three production codebases and their dependencies, at 1 and the default number of domains. make test and make test-rewatch pass.

@JonoPrest
JonoPrest added this pull request to stack #9195 November 3, 2026 11:38
JonoPrest added a commit that referenced this pull request Nov 3, 2026
The entry names #9194.

Signed-off-by: Jono Prest <jjprest@gmail.com>
@JonoPrest

Copy link
Copy Markdown
Contributor Author

@codex review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Changelog

💅 Polish

  • Build: copy the values of a dependency's interface only when a module reads them, after the first use of the same interface in a build, instead of copying all of them when the module first uses the dependency.

Semantic Diff

1 commit · Full page

Copy a dependency's values when they are read

94bd206 · JonoPrest · Nov 03, 2026, 11:26 UTC

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.
  • 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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T11:41:00.343967Z 8810538 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 8810538627

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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

codecov Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.48843% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.84%. Comparing base (eb53533) to head (94bd206).

Files with missing lines Patch % Lines
compiler/ml/env.ml 82.50% 21 Missing ⚠️
...sts/ounit_tests/ounit_deferred_components_tests.ml 94.05% 16 Missing ⚠️
Additional details and impacted files
@@                      Coverage Diff                      @@
##           jono/defer-signature-copy    #9194      +/-   ##
=============================================================
+ Coverage                      81.76%   81.84%   +0.07%     
=============================================================
  Files                            459      460       +1     
  Lines                          65448    65793     +345     
=============================================================
+ Hits                           53516    53846     +330     
- Misses                         11932    11947      +15     
Files with missing lines Coverage Δ
tests/ounit_tests/ounit_tests_main.ml 100.00% <ø> (ø)
...sts/ounit_tests/ounit_deferred_components_tests.ml 94.05% <94.05%> (ø)
compiler/ml/env.ml 81.62% <82.50%> (+1.33%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@9194

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@9194

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@9194

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@9194

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@9194

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@9194

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@9194

commit: 94bd206

@JonoPrest
JonoPrest marked this pull request as ready for review November 3, 2026 11:56
@JonoPrest
JonoPrest requested a review from cknitt November 3, 2026 11:56
@JonoPrest
JonoPrest force-pushed the jono/defer-dependency-value-copies branch from 8810538 to 67eb3bc Compare November 3, 2026 19:51
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 <jjprest@gmail.com>
@JonoPrest
JonoPrest force-pushed the jono/defer-dependency-value-copies branch from 67eb3bc to 94bd206 Compare November 3, 2026 21:51

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant