Fix infinite recursion in E.econd when optimizing nested conditionals#8039
Fix infinite recursion in E.econd when optimizing nested conditionals#8039nojaf wants to merge 3 commits intorescript-lang:masterfrom
Conversation
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Where's the self contained repro of a few lines? |
|
Can't review without a repro. |
While I understand that request, I can't reproduce it with the problematic code in a single file. I do not know what exactly trigger it, thus it must be some very unique combination. |
Can you reproduce it on the full repo, and reproduce it going away after the fix in this PR? |
|
Yep, I tried to mention this in https://github.com/rescript-lang/rescript/pull/8039/files#diff-e4faf79cc44b60806dfcf3de513d06c907723189948ac3738b02d4939252f523R109-R128 Works for that change but not for v12 stable. |
can you ask the agent to cut down your project little by little until what's left is small and still loops? |
|
We want to make sure that we're not going after a red herring. |
|
I'm able to trim some more in https://github.com/nojaf/rescript-kaplay/tree/minimal-repro It seems related to https://github.com/nojaf/rescript-kaplay/blob/minimal-repro/packages/skirmish/src/Player.res#L12
|
|
Not an infinite loop: see repro The original example is over-engineered, so it took some time to unentangle this. |
cristianoc
left a comment
cristianoc
left a comment
There was a problem hiding this comment.
See repro.
This is a perf issue not an infinite loop issue.
Ideally we want a solution that avoids the problem and does not change any of the current tests (unless there's a good reason to).
There are several ways to go about this, but prompting from a small repro should make this much easier.
|
Interesting, when I tried to reproduce this in a single file, I did't include the entire keymap variant, only a subset and thus not having the problem. Thanks a bunch for this sample! |
…mplification This PR fixes two sources of exponential compilation time: 1. **Boolean expression simplification** (`js_exp_make.ml`): The `simplify_and_` function had O(3^n) recursive behavior when simplifying nested AND expressions. Added a depth limit (10) to prevent exponential blowup while preserving optimizations for normal cases. Fixes the issue reported in #8039 and #8042 (large unboxed variants). Note: PR #8039's diagnosis of "infinite recursion" was incorrect - the actual issue was exponential blowup, not infinite loops. 2. **Exhaustiveness checking** (`parmatch.ml`): The `exhaust_gadt` function had exponential complexity (~4^n) when checking exhaustiveness for dict pattern matching. Added a call count limit (1000) that conservatively reports non-exhaustive when exceeded, preventing hangs while maintaining correctness. Performance improvements: - Large unboxed variants (28 cases): 3.79s -> 0.03s (126x faster) - Dict pattern matching (6 cases): 0.94s -> 0.04s (24x faster) Added test cases for both scenarios. Closes #8074 Closes #8039 Closes #8042
…mplification This PR fixes two sources of exponential compilation time: 1. **Boolean expression simplification** (`js_exp_make.ml`): The `simplify_and_` function had O(3^n) recursive behavior when simplifying nested AND expressions. Added a depth limit (10) to prevent exponential blowup while preserving optimizations for normal cases. Fixes the issue reported in #8039 and #8042 (large unboxed variants). Note: PR #8039's diagnosis of "infinite recursion" was incorrect - the actual issue was exponential blowup, not infinite loops. 2. **Exhaustiveness checking** (`parmatch.ml`): The `exhaust_gadt` function had exponential complexity (~4^n) when checking exhaustiveness for dict pattern matching. Added a call count limit (1000) that conservatively reports non-exhaustive when exceeded, preventing hangs while maintaining correctness. Performance improvements: - Large unboxed variants (28 cases): 3.79s -> 0.03s (126x faster) - Dict pattern matching (6 cases): 0.94s -> 0.04s (24x faster) Added test cases for both scenarios. Closes #8074 Closes #8039 Closes #8042
…mplification This PR fixes two sources of exponential compilation time: 1. **Boolean expression simplification** (`js_exp_make.ml`): The `simplify_and_` function had O(3^n) recursive behavior when simplifying nested AND expressions. Added a depth limit (10) to prevent exponential blowup while preserving optimizations for normal cases. Fixes the issue reported in #8039 and #8042 (large unboxed variants). Note: PR #8039's diagnosis of "infinite recursion" was incorrect - the actual issue was exponential blowup, not infinite loops. 2. **Exhaustiveness checking** (`parmatch.ml`): The `exhaust_gadt` function had exponential complexity (~4^n) when checking exhaustiveness for dict pattern matching. Added a call count limit (1000) that conservatively reports non-exhaustive when exceeded, preventing hangs while maintaining correctness. Performance improvements: - Large unboxed variants (28 cases): 3.79s -> 0.03s (126x faster) - Dict pattern matching (6 cases): 0.94s -> 0.04s (24x faster) Added test cases for both scenarios. Closes #8074 Closes #8039 Closes #8042
2 participants
So, my project over at nojaf/rescript-kaplay@2f34e58 was hanging this morning.
bsc.exewas in an infinite loop.The LLM and I chatted about this and we found that the problem was an infinite recursion in compiler/core/js_exp_make.ml.
I added the technical context in debug_compiler_hang.md.
@cristianoc could you take a look at this please?
I was not able to reproduce this outside of my own project.
So, this might be a quite specific edge-case 🤔