Optimizer: add a quick path before calling into freeInExpr #18895
Conversation
❗ Release notes requiredCaution No release notes found for the changed paths (see table below). Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format. The following format is recommended for this repository:
If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request. You can open this PR in browser to add release notes: open in github.dev
|
| let ``let-chains of nested apps compile under --optimize+`` depth = | ||
| let src = Gen.nestedLetApps depth | ||
| FSharp src | ||
| |> withOptions [ "--optimize+"; "--times" ] | ||
| |> asExe | ||
| |> ignoreWarnings | ||
| |> compile | ||
| |> shouldSucceed |
There was a problem hiding this comment.
Running this test on main to compare shows that the fix indeed works quite well. 5000 depth gives around 15 seconds Optimize phase on main, compared to below one sec here.
There was a problem hiding this comment.
Although this is an edge case. Hard to tell how much this will improve real life build times.
There was a problem hiding this comment.
Build fsc in Release, measure how long it takes, then recompile using the new artifacts.
|
Yeah, I don't see an improvement IRL that would stand out above the noise. I don't think the non obvious gain justifies the complication. |
2 participants
OK so this is 100% Copilot generated, to keep it consistent, here comes Copilot generated description:
This pull request introduces a more efficient and targeted way to check if a local variable is used within an expression or decision tree during optimization in the F# compiler. The main improvement is the addition of a "cheap occurs check" (
ExprUsesLocalandDecisionTreeUsesLocal) that avoids constructing full free-variable sets unless necessary, which should reduce unnecessary computation and improve performance in the optimizer. Several usages of the previous, more expensive free-variable analysis have been replaced with this new approach.Optimization and Performance Improvements
ExprUsesLocalandDecisionTreeUsesLocalto perform quick, conservative checks for variable usage within expressions and decision trees, avoiding full free-variable set construction unless required.TryEliminateBindingand related optimizer routines to use the new occurrence checks instead of always calculating free-variable sets, which should improve performance and maintain correctness.These changes collectively improve the efficiency of the optimizer, especially in large codebases, by reducing the computational cost of unused variable elimination and related analyses.## Description
This partially addresses #12421
Checklist
Test cases added
Performance benchmarks added in case of performance changes
Release notes entry updated: