Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit intermediate functions with direct method parameters instead of tuple parameters when possible #1084

Closed
4 of 5 tasks
jwosty opened this issue Oct 4, 2021 · 7 comments

Comments

@jwosty
Copy link
Contributor

jwosty commented Oct 4, 2021

In the following situation:

module MyModule 

let f1 (x:float, y:float, z:float) = x+y+z
let f2 (x:float, y:float, z:float) = x*y*z

let g flag =
    if flag then f1
    else f2

let h () =
    let func = g true
    printfn "%A" (func (4., 5., 6.))

(Sharplab.io compilation)

F# emits intermediate lambdas inside g for each function returned. These lambdas are of type FSharpFunc<Tuple<double, double, double>, double>. I propose that the compiler instead generates lambdas type either of FSharpFunc<double,double,double,double> or FSharpFunc<ValueTuple<double,double,double>,double> to avoid unnecessary heap allocations.

This can be fixed, oddly enough, by currying f1 and f2:

module MyModule 

let f1 (x:float) (y:float) (z:float) = x+y+z
let f2 (x:float) (y:float) (z:float) = x*y*z

let g flag =
    if flag then f1
    else f2

let h () =
    let func = g true
    printfn "%A" (func 4. 5. 6.)

Alternatively:

module MyModule 

let f1 (x:float, y:float, z:float) = x+y+z
let f2 (x:float, y:float, z:float) = x*y*z

let g flag =
    if flag then f1
    else f2

let h () =
    let func = g true
    printfn "%A" (func (4., 5., 6.))

(Sharplab.io compilation)

Pros and Cons

The advantages of making this adjustment to F# are:

  • Code of this form will not need to make heap allocations, which can be a problem in hot loops

The disadvantages of making this adjustment to F# are ...

  • Care would have to be taken to make this not a breaking change

Extra information

Estimated cost (XS, S, M, L, XL, XXL): M

Related suggestions: (put links to related suggestions here)

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this

For Readers

If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.

@jwosty
Copy link
Contributor Author

jwosty commented Oct 4, 2021

I would love to get some thoughts on whether or not this could be done in a backward-compatible way. If this change were to be implemented directly, it would be a problem if f1, f2, and g were compiled in an old F# compiler, and h were compiled with the newer compiler. Is there a way to get around this?

@jwosty
Copy link
Contributor Author

jwosty commented Oct 4, 2021

Also, it's good to note that another workaround is explicit ValueTuples:

module MyModule 

let f1 (struct (x:float, y:float, z:float)) = x+y+z
let f2 (struct (x:float, y:float, z:float)) = x*y*z

let g flag =
    if flag then f1
    else f2

let h () =
    let func = g true
    printfn "%A" (func (4., 5., 6.))

Also just want to elaborate that this all still applies when f1 and f2 are methods on an object instead of functions. It was surprising to me to find all of these Tuple allocations happening.

@jwosty
Copy link
Contributor Author

jwosty commented Oct 4, 2021

Potentially related to #1083

@dsyme
Copy link
Collaborator

dsyme commented Oct 5, 2021

Hi @jwosty

It's not so easy to change this, nor would I expect allocation-free programming for cases like the above. We could only enable it in some cases at considerable complexity.

It is true that iterated curried application is generally allocation-free for up to three simultaneous arguments, though that comes at the cost of a type test on the function object that can itself have runtime implications.

#1083 is not really related but I see why you linked it.

@dsyme
Copy link
Collaborator

dsyme commented Oct 13, 2021

I'll close this, partly because it's a proposed compiler optimization not a language change. ANd also because of my comment above

@dsyme dsyme closed this as completed Oct 13, 2021
@jwosty
Copy link
Contributor Author

jwosty commented Oct 13, 2021

It is true that iterated curried application is generally allocation-free for up to three simultaneous arguments, though that comes at the cost of a type test on the function object that can itself have runtime implications.

Interesting, can you elaborate on what type test it has to do? I didn't see one happening, but maybe I missed it.

@jwosty
Copy link
Contributor Author

jwosty commented Aug 26, 2023

I just realized I never actually mentioned that wasn't just a hypothetical for me; I had actually found this as the culprit to a memory problem I was having, in a real-time audio application (GC collections pausing the callback is a no-no if you can help it). I don't remember the specifics right now, but switching it to a curried function ended up fixing the GC pausing problems I was having. Had to include a comment lest I ever forget while refactoring.

I think I also misinterpreted your response as "we'll never do this;" I realize you were really saying "this doesn't belong in this repo and also it might be hard." I'll be reopening in the language repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants