[hl] Fix Out of memory on cyclic anonymous structures#12972
Open
kLabz wants to merge 3 commits into
Open
Conversation
Recursive anonymous structures (e.g. `typedef T = { next : T }` or
`{ make<S>(v:S):T }`) build tanon/HVirtual values that are cyclic in memory.
Several genhl/hlcode/hl2c lookups compared these with OCaml's polymorphic
compare, which loops forever on two *distinct* cyclic values (growing the
compare stack until Out of memory). Compiling the hl unit suite twice through
the compilation server reliably triggered it; #9662 and #12239 are
deterministic direct-compile repros.
Make the type comparisons cycle-safe without slowing genhl down:
- anons_cache: key a Hashtbl by a cheap, cycle-free hash of the anon field
names (xor of name hashes, order-independent) and resolve buckets with a
cycle-safe structural tanon_compare (De Bruijn levels break the recursion).
This both removes the loop and is at least as fast as the previous
tanon-keyed balanced map (~one structural comparison per lookup).
- ttype_compare: cycle-safe structural order on ttype (De Bruijn levels on
HVirtual); non-virtual heads short-circuit to polymorphic compare so plain
register types pay nothing. Used by mallocs, gather_types, cached_tuples,
method_wrappers, pinterfaces and the hl2c maps (defined_types, type_module,
contexts, types_descs) which loop on cyclic virtuals too.
- tsame: coinductive cycle guard.
Behaviour is unchanged for non-cyclic types, so generated bytecode is
equivalent; the hl unit suite differs only in the canonical form of one
recursive virtual (see the updated HlCodeTests.testTreeA).
Closes #9662, #12239
… types The cyclic-anonymous-structure fix made ttype_cmp cycle-safe (seen/depth) for virtual-bearing types, but ttype_compare still routed non-virtual heads -- HObj/HStruct/HEnum -- to OCaml's polymorphic `compare`, which has NO cycle detection. That is safe for current codegen only because named protos and tuples are shared singletons, so polymorphic compare short-circuits on physical equality and never walks a cycle. It is a latent footgun though: comparing two physically-distinct instances of a cyclic named type or tuple would infinite-loop -> Out_of_memory. Generalize the fix so the whole comparator is cycle-safe: route HObj/HStruct/HEnum through ttype_cmp -- named types compared by path (pname/ename, their identity), anonymous tuples (HEnum ename="") structurally via the existing cycle-safe recursion. Polymorphic compare is now used only for flat leaf kinds that cannot loop. Drop the now-unused is_virtual_bearing.
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
1 participant
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.
2nd take for #12955 which we had to revert because of poor genhl perf
Recursive anonymous structures (e.g.
typedef T = { next : T }or{ make<S>(v:S):T }) build tanon/HVirtual values that are cyclic in memory. Several genhl/hlcode/hl2c lookups compared these with OCaml's polymorphic compare, which loops forever on two distinct cyclic values (growing the compare stack until Out of memory). Compiling the hl unit suite twice through the compilation server reliably triggered it; #9662 and #12239 are deterministic direct-compile repros.Make the type comparisons cycle-safe without slowing genhl down:
Behaviour is unchanged for non-cyclic types, so generated bytecode is equivalent; the hl unit suite differs only in the canonical form of one recursive virtual (see the updated HlCodeTests.testTreeA).
Closes #9662
Closes #12256
Closes #12239