perf: preserve structural sharing for nested ledger ADT types#256
Open
RomarQ wants to merge 2 commits intoLFDT-Minokawa:mainfrom
Open
perf: preserve structural sharing for nested ledger ADT types#256RomarQ wants to merge 2 commits intoLFDT-Minokawa:mainfrom
RomarQ wants to merge 2 commits intoLFDT-Minokawa:mainfrom
Conversation
Nanopass passes auto-transform through tadt nodes, rebuilding the entire subtree at each pass. For nested Map types, this breaks structural sharing created by expand-modules-and-types, causing exponential memory growth across the analysis pass pipeline. Fix by adding: - eq?-memoized tadt clauses to cross-language passes (infer-types, remove-tundeclared, combine-ledger-declarations, recognize-let, determine-ledger-paths, remove-disclose) so shared input nodes produce shared output nodes - Pass-through tadt clauses to same-language passes (generate-contract-ht, discard-unused-functions, etc.) that return the original node unchanged Result: compilation uses constant ~80MB regardless of nesting depth, down from 3.4GB at depth 6 / OOM at depth 8. Signed-off-by: Rodrigo Quelhas <rodrigo_quelhas@outlook.pt>
RomarQ
force-pushed
the
fix/adt-nesting-preserve-sharing
branch
from
5f4b993 to
d3a27b0
Compare
Compactc E2E Tests Results 1 files ± 0 48 suites +47 2m 32s ⏱️ - 27m 27s Results for commit d3a27b0. ± Comparison against base commit fd5310c. This pull request removes 15000 and adds 467 tests. Note that renamed tests count towards both. |
Plugin Test Summary 1 files 3 suites 1s ⏱️ Results for commit fcf36c8. |
Compactc E2E Test Summary 1 files ± 0 1 suites - 47 5m 57s ⏱️ + 3m 22s Results for commit fcf36c8. ± Comparison against base commit b2a2f08. This pull request removes 467 and adds 2825 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
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.
Related to #255, instead of just adding a guard, it adds an optimization to preserve structurally shared
tadtnodes across nanopass passes.Problem
The Compact compiler crashes with OOM when compiling contracts with deeply nested
Maptypes (e.g.Map<Field, Map<Field, Map<...>>>). Depth 4 compiles fine, but depth 7 causes OOM.Each nanopass pass auto-transforms through
tadtnodes, rebuilding the entire subtree even when nothing changes. Theexpand-modules-and-typespass creates structurally sharedtadtnodes (multiple references point to the same Scheme object), but every subsequent pass breaks this sharing by creating independent copies. With ~14 passes in the analysis pipeline each independently expanding the tree, memory grows exponentially.Measured before this fix:
Fix
Add explicit
tadtclauses to the analysis passes so they preserve structural sharing instead of blindly rebuilding:eq?-keyed hash table so shared input nodes produce shared output nodes.Measured after this fix: