Repository navigation
Stop writing a copy of the .cmi into .cmt and .cmti files - #8774
Conversation
|
@codex review |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23e70a181e
ℹ️ 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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## perf-js-shake #8774 +/- ##
=================================================
- Coverage 80.66% 80.64% -0.02%
=================================================
Files 463 463
Lines 62755 62755
=================================================
- Hits 50619 50609 -10
- Misses 12136 12146 +10
🚀 New features to boost your workflow:
|
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 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". |
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
Nothing reads it; tools load the .cmi itself. Files with the copy can still be read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Christoph Knittel <ck@cca.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Christoph Knittel <ck@cca.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Christoph Knittel <ck@cca.io>
Stacked on #8773.
Like OCaml, the compiler wrote a full copy of the module's
.cmiin front of the cmt infos in every.cmti, and in every.cmtof a module without an interface. This PR stops writing it.Why the copy was there
This comes from upstream OCaml's
-bin-annotdesign. A.cmt/.cmtipair is meant to describe a compilation unit on its own: the format is "a.cmi, then the cmt infos", so a tool that only has the annotation files (documentation generators, indexers) can get both the typed tree and the exact signature with its CRCs from one file. That is why.cmtifiles always embed the.cmi(it comes from the.mli),.cmtfiles embed it only when there is no.mli, andCmt_format.readaccepts.cmi,.cmtand.cmtialike.cmt_interface_digestrecords the CRC of the embedded.cmi, so that a tool can check that an annotation file matches the interface it is used with.Why ReScript doesn't need it
.cmiis always available next to the.cmt: bsc writes it tolib/bsand rewatch copies it tolib/ocaml.Cmt_format.read_cmtand only look at the cmt infos. The one tool that needs a signature, analysis'createInterface, reads the.cmifile.cmt_interface_digest(the playground already sets it toNone).So the copy was pure cost: an extra marshalled signature in every
.cmtiand interface-less.cmt, a re-read of the partially written file to compute its digest, and an unmarshal of the copy on everyread_cmt. The only thing given up is self-containment for an external tool that reads just.cmt/.cmtiand wants the signature from them; nothing in this repository or the editor tooling does that.Compatibility
There is no format change:
Cmt_format.readalready accepts files with and without the copy (it dispatches on the leading magic number), so files from older compilers keep working, and no magic number bump is needed.cmt_interface_digestis now alwaysNone; removing the field would change the marshalled record.save_cmtno longer takes thecmi.Measurements
Runtime and Belt
lib/ocaml:.cmt(147 files).cmti(85 files).cmtfiles shrink less because modules with an interface never had the copy there.rescript-tools reanalyze -dceon the runtime package (lib/contains thelib/bscopies too: 12.2 → 9.5 MB of.cmt/.cmti), same binary, old vs. new files: identical output, major-heap words −6.7% (the unmarshalled copies), CPU time within noise since the analysis itself dominates. Compile time and allocation are unchanged (same method as #8769).🤖 Generated with Claude Code