Make module subtyping checks proportional to the expected signature, cache repeated field checks - #22281
Open
JasonGross wants to merge 3 commits into
Open
Conversation
JasonGross
force-pushed
the
fix-22279-include-self-subtyping
branch
from
July 16, 2026 14:57
189a113 to
821fb37
Compare
SkySkimmer
reviewed
Jul 16, 2026
| must be called whenever the global state is rolled back in time | ||
| (e.g. Undo, Reset, document navigation), as rolling back may remove | ||
| universe constraints that cached verdicts rely on. *) | ||
| val flush_cache : unit -> unit |
Contributor
There was a problem hiding this comment.
I don't think we want such a footgun in the kernel
Member
Author
There was a problem hiding this comment.
I had fable thread a cache instead
JasonGross
force-pushed
the
fix-22279-include-self-subtyping
branch
from
July 16, 2026 20:33
821fb37 to
e4a7c10
Compare
JasonGross
force-pushed
the
fix-22279-include-self-subtyping
branch
from
July 16, 2026 21:00
e4a7c10 to
139ee27
Compare
JasonGross
marked this pull request as ready for review
July 16, 2026 23:05
Member
|
This still doesn't look like the correct way to fix this, even though I haven't given a lot of thought about a potential solution yet. |
Subtyping.check_subtypes used to eagerly strengthen the whole signature of the subtype side and to eagerly build a label map over all its fields, even when the expected signature only requires checking a few fields (or none). Chaining functor module types with <+ re-runs this check on the whole accumulated signature at every stage (twice: once for universe inference and once in the kernel), which made such chains quadratic in the total number of fields (rocq-prover#22279). All non-checker callers guarantee that the fields of the subtype side are already part of the environment, unsubstituted (this is the invariant documented in check_structure for the non-functor case). Expose a [direct] flag on check_subtypes for those callers: fields are then looked up in the environment by name on demand, and constant and submodule fields are strengthened individually at lookup time (mirroring what Modops.strengthen produces), instead of walking the whole signature. Lookups fall back to a lazily-built label map when a label does not name a field of the module (e.g. constructor names or secondary inductive types of a block), preserving behavior in those cases, in the checker, and in the recursive cases where the environment holds substituted fields. On the benchmark from rocq-prover#22279 (800 chained interfaces of 10 parameters each over an empty parameter signature), the Module Type command goes from 3.33s to 0.15s; with a shared 3-field parameter signature it goes from 0.46s to 0.05s (n=800, 2 fields per interface). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_019ttctspSoVoquHLQtbPVZw
Some module constructions legitimately perform quadratically many field subtype checks of the same pairs of fields: chains of module types each parameterized by a prefix of the chain, or repeated applications of the same functor. Cache successful checks of a constant field pair under conditions, checked at run time, that make the verdict a function of the pair alone: both substitutions leave the bodies physically unchanged, and the check produced no new universe constraints (the returned state is physically the input one — always true of the kernel checking mode, and true of the inference mode exactly when the needed constraints are already entailed). Cached verdicts stay valid as long as the ambient environment evolves monotonically, so the cache is purely functional and lives alongside the environment it was computed in: threaded through the module translation functions and stored in a field of the kernel's safe_environment on the checking side, and in a summary-registered reference on the inference side (Declaremods). Discarding or rolling back the state (Undo, Reset, document navigation) thereby discards or rolls back the cache with it — there is no global mutable state and no invalidation obligation. The cache is indexed by field label with a short capped list of physically-compared pairs, so lookups are cheap and retained memory is bounded. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_019ttctspSoVoquHLQtbPVZw
Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_019ttctspSoVoquHLQtbPVZw
JasonGross
force-pushed
the
fix-22279-include-self-subtyping
branch
from
July 21, 2026 00:04
139ee27 to
f58b5e5
Compare
5 tasks
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.
Fixes #22279 (quadratic time when chaining module types with
<+).Written by Claude (Anthropic AI) at the request of and under the supervision of @JasonGross.
Problem
Every
<+of a functor module type triggers the "Include Self" subtyping check of the whole accumulated signature against the functor's parameter signature, twice (once with universe inference inDeclaremods, once in the kernel).Subtyping.check_subtypesdid two things proportional to the size of the subtype side, even when the expected signature needs few or no fields checked:strengtheneagerly rebuilt every accumulated constant field;check_signatureseagerly built a label map (make_labmap) over the whole accumulated signature.For a chain of n interfaces this is O(n²) with significant constants. Instrumented at n=800 (10 parameters per interface, empty parameter signature): 3.3s of CPU in the single
Module Type ... := I1 <+ ... <+ Incommand, ~1.6s instrengthen, ~2.3s inmake_labmap, across 1600check_subtypescalls.Fix
Commit 1 — make each check proportional to the expected signature. All non-checker callers of
check_subtypesguarantee that the subtype side's fields are already in the environment, unsubstituted (the invariant documented incheck_structurefor thenargs = 0case). A newdirectflag makescheck_signatureslook fields up in the environment by name on demand, strengthening constant and submodule fields individually at lookup time (mirroring exactly whatModops.strengthenproduces per field). Lookups fall back to the lazily-built label map whenever the label does not name a field (constructor names, secondary inductive types of a block), preserving existing behavior, error messages, the checker paths, and the recursive cases where the environment holds substituted fields.Commit 2 — cache successful field checks. Prefix-shaped chains (each stage parameterized by the whole chain so far) and repeated applications of the same functor legitimately perform quadratically many field checks. Successful checks of a constant field pair are now cached under conditions that make the verdict a function of the pair alone, all checked at run time: both substitutions leave the bodies physically unchanged, and the check produced no new universe constraints (the returned state is physically the input one — true of the kernel checking mode always, and of the inference mode exactly when the needed constraints are already entailed). Cached verdicts stay valid while the environment grows monotonically, so the cache is purely functional and lives alongside the environment it was computed in: it is threaded through
Mod_typing/Subtypingand stored in a field of the kernel'ssafe_environmenton the checking side, and in a summary-registered reference on the inference side (Declaremods). Discarding or rolling back the state (Undo, Reset, document navigation) thereby discards or rolls back the cache with it — there is no global mutable state in the kernel and no invalidation obligation on callers. The cache is indexed by field label with a short capped list of physically-compared pairs, so lookups are cheap and retained memory is bounded.Measurements
(
Module Typecommand time from-time, or wall time for whole files; baseline is currentmaster)ArgsArgsTests
test-suite/modules/IncludeSelfChain.vcovers chains over empty and non-empty parameter signatures (including definitions checked up to conversion, inductive types and submodules in the parameter signature), rejection of ill-typed chains, prefix-shaped chains, repeated functor applications, and aReset-and-redo of a check whose universe constraints must be reproduced (exercising the cache flush). Themodules,success,failure,bugs, andoutputtest-suite subsystems pass.🤖 Generated with Claude Code
https://claude.ai/code/session_019ttctspSoVoquHLQtbPVZw