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

Introduce a InferAsValueTuples=X compiler flag. #813

Open
OnurGumus opened this issue Dec 1, 2019 · 16 comments
Open

Introduce a InferAsValueTuples=X compiler flag. #813

OnurGumus opened this issue Dec 1, 2019 · 16 comments

Comments

@OnurGumus
Copy link

@OnurGumus OnurGumus commented Dec 1, 2019

Introduce a compiler switch as InferAsValueTuples = X

I propose we introduce a compiler switch to change the default inference of tuples as value tuples where InferAsValueTuples = X will cause all tuples with length less than X being inferred as ValueTuples.
The default value of X will be 0 and will effectively turn off this feature.

The existing way of approaching this problem in F# is to use struct(x,y) but many times the users don't use the struct keyword and this creates the following problems

  • It requires more typing.
  • It creates a false sense of correctness as in ref tuples are better defaults since arguably especially for smaller tuples, ValueTuples perform better and are better defaults.

C# people are already using ValueTuples by default.

Pros and Cons

The advantages of making this adjustment to F# are

  • People will use saner default as ValueTuples

  • Less typing and easier reading

  • Better C# ValueTuple interop

  • Gradually increase the default value of X if needed.

The disadvantages of making this adjustment to F# are

  • It may cause some confusion when different projects use different values for X but source will still compile no matter what.
  • At source code level it may break things as well as at binary level.
  • The author of the code has to remember what X is, but still he will get a meaningful error message if something goes wrong.
  • If you really need a ref tuple in all cases you must give the type hint explicitly.
    let (x,y):(int*int) = 3,5

Extra information

S

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
@OnurGumus OnurGumus changed the title Introduce a InferValueTuples=X compiler flag. Introduce a InferAsValueTuples=X compiler flag. Dec 1, 2019
@cartermp

This comment has been minimized.

Copy link
Member

@cartermp cartermp commented Dec 1, 2019

At source code level it won't break anything

This is not true. The following code will fail to compile if x is inferred to be a ValueTuple.

let x = (1, 2)
let y = x.ToValueTuple()

X would also have to be the size in bytes, not the number of elements in a tuple, making the ergonomics of this suggestion a bit more difficult.

@OnurGumus

This comment has been minimized.

Copy link
Author

@OnurGumus OnurGumus commented Dec 1, 2019

@cartermp that's not what I meant. You don't need that method, if you have any api consumes ref tuples it will break anyway :) What I meant was, since default would be zero, adding this flag won't break anything.

@Happypig375

This comment has been minimized.

Copy link
Contributor

@Happypig375 Happypig375 commented Dec 1, 2019

Before adding this, #782 would need to be addressed to make this feature more usable.

@cartermp

This comment has been minimized.

Copy link
Member

@cartermp cartermp commented Dec 1, 2019

@OnurGumus While it's true that one wouldn't need that method, that doesn't change the fact that this suggestion is a breaking change. A valid F# program today that calls this method on a reference tuple will fail to compile if this were implemented in the next F# language version, thus making this a source-breaking change.

@OnurGumus

This comment has been minimized.

Copy link
Author

@OnurGumus OnurGumus commented Dec 2, 2019

@cartermp OK I moved it to disadvantages stating it may break things.

@varon

This comment has been minimized.

Copy link

@varon varon commented Dec 9, 2019

Please can we get this in.

@abelbraaksma

This comment has been minimized.

Copy link

@abelbraaksma abelbraaksma commented Dec 9, 2019

I'm not sure how this could possibly work. Value tuples and normal tuples have a disjunct set of functions that apply to them. Also, any member of a bcl class takes a reference tuple, as do table style match patterns. Changing this and making it work with or without this switch seems a huge undertaking, by no means 'S', or perhaps I misunderstand the suggestion.

That's not even mentioning the possible negative impact on performance, or running out of stack space (value tuples can take up significantly more stack space, iirc). Carefully crafting a balance between value tuples and reference tuples is at the heart of good performance tuning. A switch to turn reference tuples magically into value tuples is not gonna change that, if anything, it would make tuning more complex.

As an exercise, try to code with #if INFERVALUETUPLES and write the different code paths by hand in a test project, to find out if it is at all feasible to automate this process (it's what a compiler would need to do). I think it isn't, but I'd happily be proven wrong.

@TIHan

This comment has been minimized.

Copy link

@TIHan TIHan commented Dec 10, 2019

Adding a switch that can change language semantics like this is a slippery slope. One project could use ValueTuple where as another would be Tuple; harder to reason about between projects. It's better to stay consistent with what we have.

I understand a lot of folks do like ValueTuple for C# and wish F# could have that by default. However, the practice of using tuples in C# and F# are slightly different. In F#, you tend to pass Tuples around more often than C#. If F# defaulted to ValueTuple, you could be incurring extra copying and getting worse performance as a result. Also, F# can de-tuple expressions in some cases; therefore, no allocation of any kind.

@7sharp9

This comment has been minimized.

Copy link
Member

@7sharp9 7sharp9 commented Dec 10, 2019

ValueTuples perform better and are better defaults

This has proven to not be the case.

Tuples are far less prevalent in C# so I can understand the default there but not in F#.

@varon

This comment has been minimized.

Copy link

@varon varon commented Dec 14, 2019

Rather than absolute throughput, do note that some tasks and workloads are incredibly sensitive to garbage which forces us to write struct all over the place.

Avoiding this with clean syntax (even at a possible throughput cost in some edge cases) would be a great improvement.

@abelbraaksma

This comment has been minimized.

Copy link

@abelbraaksma abelbraaksma commented Dec 14, 2019

How is struct not clean and clear? It conveys the intend and meaning and is only a few keystrokes long. Or you can use type aliases for more clarity and brevity.

But apart from explaining the status quo, which I'm sure you're aware of, assuming we would want this in, I don't see any technical way of getting it done apart from breaking code. And even if you'd allow a switch that changed behavior (which this does), how can the compiler possibly understand when you need the reference version of a tuple? Don't forget, as explained before, the whole language is designed around tuples. Would you suggest that if we lose struct, we introduce a keyword to mean 'reference tuple'?

Either way, so far I've not seen a suggestion to do this without breaking code and behavior, and without very significant changes to the language as a whole.

@cartermp

This comment has been minimized.

Copy link
Member

@cartermp cartermp commented Dec 15, 2019

Rather than absolute throughput, do note that some tasks and workloads are incredibly sensitive to garbage which forces us to write struct all over the place.

We're well aware. But it's also worth noting that just because struct is one something, doesn't mean it won't generate lots of garbage.

struct (Array.zeroCreate<byte> 85_000, Array.zeroCreate<byte> 85_000) will allocate twice on the large object heap, which can lead to the GC to pause all threads as it cleans up data.

@Happypig375

This comment has been minimized.

Copy link
Contributor

@Happypig375 Happypig375 commented Dec 15, 2019

That will be 3 allocations wirh reference tuples compared to 2 allocations with struct tuples, won't it? One less allocation is still better, although not having a large effect on overall performance.

@abelbraaksma

This comment has been minimized.

Copy link

@abelbraaksma abelbraaksma commented Dec 15, 2019

That will be 3 allocations wirh reference tuples

@Happypig375 Why? Do you mean that creates three large arrays? I don't think so. Both struct and reference tuples will create the arrays on the LO heap. I'd be surprised if there's a difference when using struct vs reference tuples where the contents contains references.

I may be wrong, but unless you use struct tuples with struct data, the speed gain or resources gain will be minimal. Though I'd be interested in a serious benchmark that backs this.

@Happypig375

This comment has been minimized.

Copy link
Contributor

@Happypig375 Happypig375 commented Dec 15, 2019

@abelbraaksma

One less allocation is still better, although not having a large effect on overall performance.

🙂

@cartermp

This comment has been minimized.

Copy link
Member

@cartermp cartermp commented Dec 15, 2019

That will be 3 allocations wirh reference tuples compared to 2 allocations with struct tuples, won't it? One less allocation is still better, although not having a large effect on overall performance.

Perf-wise it's noise, so it doesn't matter (assuming the compiler doesn't optimize away the tuple in the first place).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.