Skip to content

Replace compiler CPPO specializations with functors - #8542

Merged
cristianoc merged 1 commit into
masterfrom
replace-cppo-specializations-with-functors
Aug 10, 2026
Merged

Replace compiler CPPO specializations with functors#8542
cristianoc merged 1 commit into
masterfrom
replace-cppo-specializations-with-functors

Conversation

@cristianoc

Copy link
Copy Markdown
Collaborator

Replace the compiler's six data-structure CPPO templates and their Dune generation rules with ordinary OCaml functors and small, explicit instantiation modules. Keep Hash_set_poly as a plain polymorphic implementation because its key type is not fixed by a functor argument.

Audit every resulting module against production consumers. Remove the unused Hash_set_int specialization and the effectively dead Hash_string specialization, whose only apparent compiler consumer was an always-empty cache for a commented-out function. Remove that cache and its uncalled Ext_ident.reset API; tests instantiate Hash.Make locally where a string-key table is useful.

Remove CPPO from the analysis and tools package dependencies. Retain it in the rescript package, with an explanatory comment, because the runtime's explicit regeneration script still invokes the external CPPO executable for its checked-in ReScript specializations.

This makes every compiler implementation visible to editors and build tooling, removes generated-source indirection, and uses the language's standard abstraction mechanism while preserving each retained specialization's comparison, equality, hashing, and printing semantics.

A four-way OCaml 5.3 release-build experiment compared concrete CPPO and functor implementations, both with and without Flambda. No meaningful compiler-speed difference was observed between CPPO and functors on clean builds of the 150-module runtime corpus, parallel builds, or incremental rebuilds; generated JavaScript was identical. Isolated data-structure benchmarks did show differences without Flambda, but those differences disappeared with Flambda.

Flambda itself was slower for representative compilation workloads while improving several sustained parsing and printing benchmarks. Its larger binary and startup cost appear to contribute to the typical-build slowdown, although startup-only measurements do not explain all of it. Investigating Flambda further remains worthwhile, particularly together with a persistent compiler server that can amortize per-file process startup.

Add direct coverage for the ordered local-identifier map and retain the existing map, set, hash, hash-set, and vector tests.

Signed-off-by: Cristiano Calcagno cristianoc@users.noreply.github.com


Stack created with GitHub Stacks CLIGive Feedback 💬

@cristianoc
cristianoc force-pushed the replace-cppo-specializations-with-functors branch from 571d407 to 6beded4 Compare August 10, 2026 09:34
@cristianoc
cristianoc marked this pull request as ready for review August 10, 2026 09:34
@cristianoc
cristianoc requested a review from cknitt August 10, 2026 09:35
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.67176% with 117 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.83%. Comparing base (9f26fa6) to head (c9fa8c5).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
compiler/ext/vec.ml 67.94% 50 Missing ⚠️
compiler/ext/ext_map.ml 66.66% 31 Missing ⚠️
compiler/ext/hash.ml 68.33% 19 Missing ⚠️
compiler/ext/ordered_hash_map.ml 76.74% 10 Missing ⚠️
compiler/ext/hash_set.ml 77.27% 5 Missing ⚠️
compiler/ext/ext_set.ml 97.95% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8542      +/-   ##
==========================================
+ Coverage   74.70%   74.83%   +0.13%     
==========================================
  Files         460      477      +17     
  Lines       62389    63311     +922     
==========================================
+ Hits        46608    47380     +772     
- Misses      15781    15931     +150     
Files with missing lines Coverage Δ
compiler/ext/ext_ident.ml 72.72% <ø> (+0.66%) ⬆️
compiler/ext/hash_ident.ml 100.00% <100.00%> (ø)
compiler/ext/hash_set_ident.ml 100.00% <100.00%> (ø)
compiler/ext/hash_set_poly.ml 100.00% <100.00%> (ø)
compiler/ext/ordered_hash_map_local_ident.ml 100.00% <100.00%> (ø)
compiler/ext/set_ident.ml 100.00% <100.00%> (ø)
tests/ounit_tests/ounit_hash_stubs_test.ml 100.00% <ø> (+28.57%) ⬆️
tests/ounit_tests/ounit_hashtbl_tests.ml 100.00% <100.00%> (ø)
tests/ounit_tests/ounit_scc_tests.ml 84.33% <100.00%> (ø)
compiler/ext/ext_set.ml 97.95% <97.95%> (ø)
... and 5 more

... and 4 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 Aug 10, 2026

Copy link
Copy Markdown

Open in StackBlitz

rescript

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

@rescript/darwin-arm64

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

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8542

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

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

@rescript/win32-x64

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

commit: c9fa8c5

@cknitt

cknitt commented Aug 10, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 6beded4738

ℹ️ 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".

Base automatically changed from browser-platform-modules to master August 10, 2026 14:42
Replace the compiler's six data-structure CPPO templates and their Dune generation rules with ordinary OCaml functors and small, explicit instantiation modules. Keep Hash_set_poly as a plain polymorphic implementation because its key type is not fixed by a functor argument.

Audit every resulting module against production consumers. Remove the unused Hash_set_int specialization and the effectively dead Hash_string specialization, whose only apparent compiler consumer was an always-empty cache for a commented-out function. Remove that cache and its uncalled Ext_ident.reset API; tests instantiate Hash.Make locally where a string-key table is useful.

Remove CPPO from the analysis and tools package dependencies. Retain it in the rescript package, with an explanatory comment, because the runtime's explicit regeneration script still invokes the external CPPO executable for its checked-in ReScript specializations.

This makes every compiler implementation visible to editors and build tooling, removes generated-source indirection, and uses the language's standard abstraction mechanism while preserving each retained specialization's comparison, equality, hashing, and printing semantics.

A four-way OCaml 5.3 release-build experiment compared concrete CPPO and functor implementations, both with and without Flambda. No meaningful compiler-speed difference was observed between CPPO and functors on clean builds of the 150-module runtime corpus, parallel builds, or incremental rebuilds; generated JavaScript was identical. Isolated data-structure benchmarks did show differences without Flambda, but those differences disappeared with Flambda.

Flambda itself was slower for representative compilation workloads while improving several sustained parsing and printing benchmarks. Its larger binary and startup cost appear to contribute to the typical-build slowdown, although startup-only measurements do not explain all of it. Investigating Flambda further remains worthwhile, particularly together with a persistent compiler server that can amortize per-file process startup.

Add direct coverage for the ordered local-identifier map and retain the existing map, set, hash, hash-set, and vector tests.

Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
@cristianoc
cristianoc force-pushed the replace-cppo-specializations-with-functors branch from 6beded4 to c9fa8c5 Compare August 10, 2026 14:42
@github-actions

Copy link
Copy Markdown

@cristianoc
cristianoc merged commit 6a6ef66 into master Aug 10, 2026
29 checks passed
@cristianoc
cristianoc deleted the replace-cppo-specializations-with-functors branch August 10, 2026 15:30
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.

2 participants