perf: memoise per-(dep_m, ml_kind, cm_kind, is_consumer) raw-refs builder#14474
perf: memoise per-(dep_m, ml_kind, cm_kind, is_consumer) raw-refs builder#14474robinbb wants to merge 1 commit into
Conversation
|
Carrying over Copilot's review comment from #14454 for continued discussion here. Flags potential unsoundness in
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a per-compilation-context memoization layer for the “raw module reference set” Action_builder.t used by Module_compilation.lib_deps_for_module, aiming to reduce repeated Action_builder construction when multiple modules share overlapping trans_deps and when the code runs in both Impl and Intf passes.
Changes:
- Add
Compilation_context.cached_raw_refsto cache raw-ref builders by(dep_m, ml_kind, cm_kind, is_consumer)within a singlecctx. - Switch
Module_compilation.lib_deps_for_moduleto usecached_raw_refs, normalizingml_kindfortrans_depsto share the cache betweenImpl/Intfpasses. - Document and expose the cache entrypoint in
compilation_context.mli.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/dune_rules/module_compilation.ml | Routes raw-ref computation through the new cctx cache to avoid repeated builder construction. |
| src/dune_rules/compilation_context.mli | Exposes and documents cached_raw_refs as a Compilation_context API. |
| src/dune_rules/compilation_context.ml | Implements the per-cctx raw-ref cache (Raw_refs) and stores it on the cctx record. |
robinbb
force-pushed
the
robinbb-memoise-intra-stanza
branch
from
0453498 to
335a6cc
Compare
robinbb
force-pushed
the
robinbb-issue-4572-filtered-includes
branch
6 times, most recently
from
a97c222 to
f28a9ca
Compare
robinbb
force-pushed
the
robinbb-memoise-intra-stanza
branch
2 times, most recently
from
b21618c to
f9b5b61
Compare
[lib_deps_for_module] rebuilt a fresh [read_dep_m_raw] [Action_builder.t] per (consumer, dep_m) pair. Sibling consumers share most of [trans_deps], so the wrapping [need_impl_deps_of] / [Module_name.Set.union] logic was reconstructed N×K times per stanza ([Ocamldep] already shared the inner read via its path-keyed cache). Add a per-cctx [Table.t] keyed on a [Raw_refs.Key.t] variant: [Direct] carries [ml_kind] (consumer rows), [Transitive] carries [cm_kind] (trans-dep rows). Each case omits the dimensions irrelevant to its closure behaviour, so cache entries collapse maximally; [Table.find] short-circuits before allocating. Reduces Action_builder allocations per stanza from O(N×K) to O(N). Per-consumer iteration over [trans_deps] is unchanged (bind count still O(N×K)); a recursive O(N+E) aggregate is a follow-up. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-issue-4572-filtered-includes
branch
from
f28a9ca to
33208f1
Compare
robinbb
force-pushed
the
robinbb-memoise-intra-stanza
branch
from
f9b5b61 to
7cb3a43
Compare
Replace the cctx-wide library closure with a per-module dep set computed from ocamldep output. A consumer module's compile rule sees only the artifacts of libraries it actually references, so unrelated sibling modules no longer recompile when an unreferenced dependency library's cmi changes. User-facing detail is in the shipped changelog entry (doc/changes/added/14491.md). Components (each was a now-closed predecessor PR): - Per-module library dep filter via ocamldep BFS (was ocaml#14472). - Per-module -I/-H include flags matching the filter (was ocaml#14473). - Raw_refs Action_builder cache (was ocaml#14474) to deduplicate ocamldep results across cctxs sharing the same module sources. - Filtered_includes Action_builder cache to share rule deps between modules with identical kept-libs sets. Fixes ocaml#4572. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Replace the cctx-wide library closure with a per-module dep set computed from ocamldep output. A consumer module's compile rule sees only the artifacts of libraries it actually references, so unrelated sibling modules no longer recompile when an unreferenced dependency library's cmi changes. User-facing detail is in the shipped changelog entry (doc/changes/added/14491.md). Components (each was a now-closed predecessor PR): - Per-module library dep filter via ocamldep BFS (was ocaml#14472). - Per-module -I/-H include flags matching the filter (was ocaml#14473). - Raw_refs Action_builder cache (was ocaml#14474) to deduplicate ocamldep results across cctxs sharing the same module sources. - Filtered_includes Action_builder cache to share rule deps between modules with identical kept-libs sets. Fixes ocaml#4572. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Summary
Stacked atop #14473. Per-cctx Table cache for the
read_dep_m_rawAction_builder.t; reduces per-stanza Action_builder allocations from O(N×K) to O(N). Bind count unchanged. Follow-up PR pending for the recursive O(N+E) shape.Prerequisite PRs