KEMBAR78
Optimize simple range mappings: `[for n in start..finish -> f n]`, &c. by brianrourkeboll · Pull Request #16832 · dotnet/fsharp · GitHub
Skip to content

Conversation

@brianrourkeboll
Copy link
Contributor

@brianrourkeboll brianrourkeboll commented Mar 7, 2024

Description

  • Apply the optimizations from Better integral range lowering: start..finish, start..step..finish #16650 to lists and arrays constructed from simple, direct range mappings, i.e. —

    • [for n in start..finish -> n]
    • [for n in start..finish -> f n]
    • [for n in start..step..finish -> n]
    • [for n in start..step..finish -> f n]
    • [|for n in start..finish -> n|]
    • [|for n in start..finish -> f n|]
    • [|for n in start..step..finish -> n|]
    • [|for n in start..step..finish -> f n|]

    — where f stands for any arbitrary transformation of n.

    This also applies to the syntax [for n in start..finish do yield n], etc., when there is exactly one yield, and to [for n in start..finish do f (); …; yield n], for any depth of sequential expressions, as long as there is still exactly one yield.

    The generated loop IL for [for n in start..finish -> n], etc., is identical to that generated for [start..finish].

Benchmarks

The performance improvement looks to be the same as that in #16650 (i.e., up to 8×), as would be expected, since it's the same count and initialization loop code. I can run a more thorough suite if desired.

| Categories                                                                      | Mean      | Ratio | Gen0    | Gen1    | Gen2    | Allocated  | Alloc Ratio |
|-------------------------------------------------------------------------------- |----------:|------:|--------:|--------:|--------:|-----------:|------------:|
| Array,[|for n in start..step..finish -> n * n|] (start=1,step=2,finish=65536)   | 205.05 us |  1.00 | 83.2520 | 83.2520 | 83.2520 |  384.49 KB |        1.00 |
| Array,[|for n in start..step..finish -> n * n|] (start=1,step=2,finish=65536)   |  43.95 us |  0.21 | 41.6260 | 41.6260 | 41.6260 |  128.04 KB |        0.33 |
|                                                                                 |           |       |         |         |         |            |             |
| List,[for n in start..step..finish -> n * n] (start=1,step=2,finish=65536)      | 197.15 us |  1.00 | 83.4961 | 71.7773 |       - | 1024.09 KB |        1.00 |
| List,[for n in start..step..finish -> n * n] (start=1,step=2,finish=65536)      | 158.13 us |  0.80 | 83.4961 | 71.5332 |       - |    1024 KB |        1.00 |

Checklist

  • Test cases added
  • Performance benchmarks added in case of performance changes
  • Release notes entry updated

Notes

This PR also optimizes simple mappings comprised of one or more sequential expressions like [for n in start..finish do printfn "abc"; yield n], where there is exactly one yield (5d52462). We could theoretically go even further and detect a statically-known number of yields > 1 and factor that back into the total iteration count, but that seems like a step too far for now.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2024

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/8.0.300.md

@brianrourkeboll brianrourkeboll marked this pull request as ready for review March 8, 2024 17:05
@brianrourkeboll brianrourkeboll requested a review from a team as a code owner March 8, 2024 17:05
@T-Gro T-Gro enabled auto-merge (squash) March 18, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants