-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Initial blittable proposal #206
Conversation
CC @dotnet/csharplangdesign |
M<User>(); // Error: Type User does not satisfy the blittable constraint. | ||
``` | ||
|
||
One of the primary motivations for `blittable` structs is ease of interop. As such it's imperative that such a type have it's field ordered in a sequential, or explicit, layout. An auto layout of fields makes it impossible to reliably interop the data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: second "it's" -> "its"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its correct I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rule 1: When you mean it is or it has, use an apostrophe. Rule 2: When you are using its as a possessive, don't use the apostrophe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, rule of thumb I say is if it has a apostrophe, read it in your head as "it is" and see it if makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jnm2 *and see it if it 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simple mneumonic for its/it's: Think of the apostrophe as the dot on an 'i'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GaTechThomas mnemonic
(I'm well aware that I'm in a glass house re: typos 😂)
The lack of constraints here also make it impossible to have efficient conversions between streams of data and more structured types. This is an operation that is common in networking stacks and serialization layers: | ||
|
||
``` c# | ||
Span<byte> Convert<T>(ref T value) where T : blittable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the constraint be blittable struct
, or is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Should've been blittable struct.
The lack of compile time support also makes it difficult to abstract over blittable types. It's not possible for instance to author common helper routines using generic code: | ||
|
||
``` c# | ||
void Hash<T>(T value) where T : blittable struct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: You use blittable struct
here and blittable
below. We should probably be consistent with one or the other.
I personally prefer just blittable
, since you can't have a blittable
class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will make it consistent. My preference is still for blittable struct
though because I want to allow for other future features. A class
may not be blittable
but what about a static delegate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredpar can you say "blittable type" to cover all possible blittable types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@whoisj not quite sure what you are asking there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredpar seemed like confusion of using blittable struct
vs blittable
, I was just suggesting use of the word "type" in place of "struct" to keep it vague but accurate. 'Tis all. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredpar, a static delegate
, as currently proposed is a blittable struct
😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this allow the use of generic pointers? Like: void Hash<T>(T* pointer) where T : blittable struct
|
||
The compiler will enforce that the fields of such a struct definition fit one of the following categories: | ||
|
||
- `sbyte`, `byte`, `short`, `ushort`, `int`, `uint`, `long`, `ulong`, `char`, `float`, `double`, `decimal`, or `bool`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, Please, Please do not leave off IntPtr
and UIntPtr
. These types are always blittable and are one of the core interop types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was mentioned in the issue as well. It was an ommission, will add before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your list contains quite a few non blittable types, e.g. decimal, char, bool. Are those mistakes or so you plan to change this?
If you plan to change this, what about guid, datetime, datetimeoffset, timespan?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decimal
, Guid
, DateTime
, DateTimeOffset
, and TimeSpan
are implicitly blittable as they are structs composed of blittable types.
char
and bool
are in the category of "sometimes blittable", so we should be careful about including them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decimal, Guid, DateTime, DateTimeOffset, and TimeSpan are implicitly blittable as they are structs composed of blittable types.
I've waffled a bit on whether these should be special cased as implicitly blittable, or waiting for source annotations to happen. Leaning towards implicit.
char and bool are in the category of "sometimes blittable", so we should be careful about including them.
Been way down this rabbit hole in Midori. The basic problem comes down to whether you believe bilttable means ...
- A type which has no field, at any level of nesting, whose type is a reference type
- 1 + the constraint that all possible bit patterns represent a valid instance of the type
In Midori we actually separated this out into two features: primitive and blittable. All blittable are primitive but not the other way around. The distinction was interesting but IMHO wasn't worth the extra complexity.
In any case though, this feature definitely targets 1 above. It's the language manifestation of the existing spec concept of "unmanaged type".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if we are explicitly stating that this is for 'unmanaged types' (that is types for which the unmanaged and managed representation is bit for bit identical) then we should not be including 'char' or 'bool', since the statement is not always true for these types.
String is technically blittable in some cases (and there is logic for pinning a string rather than copying in those instances).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tannergooding DateTime
and DateTimeOffset
are currently not implicitly blittable as they are structs marked with layout Auto, even though they are composed of blittable types.
I guess that would need fixing for this proposal, otherwise this might exclude quite a few user defined blittable types (especially for serialization).
Still a pitty that nothing really can be done for bool
besides using a classic BlittableBool struct wrapper.
Thank you both for your detailed explanations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, I missed the Auto layout attribute. It might be worth logging bugs to remove that. At least in the case of DateTime
, it currently only contains a single field (a UInt64
) and will have the same layout regardless. For DateTimeOffset
, it contains a DateTime
and a Int16
, so the layout should be the same as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed bugs https://github.com/dotnet/coreclr/issues/10448 and https://github.com/dotnet/coreclr/issues/10449 for the layoutkind issue
- Any pointer type which points to a `blittable` type | ||
- Any user defined struct explicitly declared as `blittable` | ||
|
||
Any type fitting the definition above, or any element in the list, is considered a valid `blittable` type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought you wanted to add a section for fixed arrays as well (#78)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered that but as we were discussing I'm not sure why we should limit this to blittable types, seems we can trivially expand it to any type out there. Working to validate if that's legal or not. If it's not then I'll go back and expand this proposal to do that.
I should explicitly mention in the proposal though that a fixed array of blittable types is considered a valid field type inside a blittable struct.
@jaredpar, what type of 'contract' are we attempting to declare with Are we also saying that:
I think these are important questions, partially because some things are still safe, provided the consumer is aware they can happen. For example, exposing new members on the end of a struct is technically OK, provided the user isn't assuming a fixed size. Also:
This is probably important because in the first means the user can view the direct field layout, where-as the latter means someone can abstract it away if the need/want. Some of this may also become important when tied in with Static Delegates (#126) as users can manually implement VTBLs for interop code (and may want to mark those as blittable). |
@tannergooding I would expect the contract purely to assert the data layout is the same between managed and unmanaged code, and maintaining those other assumptions would still be the responsibility of the programmer, if that's what they need. I haven't looked at #126 yet though, so maybe that context would shift my thinking. |
There are no restrictions on future changes to the For instance most developers wouldn't think twice about adding a field to a struct which already has at least one public constructor. Not really going to break anyone with that. Adding it to a blittable struct though is more likely to raise the correct questions from reviewers and the developer in question.
Visibility of fields does not factor into whether a struct can / can't be marked as |
👍 That is about what I expected |
Maybe use keyword 'fixed' instead of adding 'blittable' which is not a keyword and is really jargon-y. |
Personally, I think |
Fixed already sort of implies/means blittable in C#. The keyword fixed is used on fixed sized array fields, making them inline to the struct and can only have blittable element types. A fixed field is blittable. An entire fixed struct would be the same. |
That is, |
@tannergooding I'm assuming any proposal to add blittable structs would include changing fixed arrays to include these types as well. |
I'm honestly fine with Seriously That said, can we please get an Please please please, with a cherry on top? 🍨 🍒 |
|
|
||
- `sbyte`, `byte`, `short`, `ushort`, `int`, `uint`, `long`, `ulong`, `char`, `float`, `double`, `decimal`, or `bool`. | ||
- Any `enum` type | ||
- Any pointer type which points to a `blittable` type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't this be any pointer? It should not matter what the pointer points to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also void*
should be blittable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I withdraw my flat
proposal :)
Yet these frameworks are are often the basis for the majority of .NET applications out there. Hence performance / correctness wins at this level can have a riple effect on the .NET ecosystem. This makes the feature worth considering even with the limited audience. | ||
|
||
There will also likely be a small transition period after this is released where core libraries move to adopt it. Types like [System.Runtime.InteropServices.ComTypes.FILETIME](https://msdn.microsoft.com/en-us/library/system.runtime.interopservices.comtypes.filetime(v=vs.110).aspx) are common in interop code. Until it is marked as `blittable` in source though, developers won't be able to depend on it in their libraries. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very important feature for Unity, that goes far beyond low level library authors. This is something many of our customers would use. In games generally we very much like the ability to pack our memory in a very controlled way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once we have blittable
we can start discussing things like an IFixable<T>
interface where T : blittable
, which would enable fixed (void* p = myIFixable)
and other useful syntax candy.
While I like the idea to be able to explicitly mark a struct with a blittable attributes/keyword so that compile time checking is not only done on generic bindings but also on struct declaration itself, but the fact that the implicit behavior is not possible is quite annoying, as it will require to go over all existing structs that are blittable today and add this attribute. By introducing the keyword, it will not change the fact that the JIT/AOT will always check that the struct is actually blittable, even if the method has a generic "blittable" constraint, it will have to check the type. Why couldn't we allow both, implicit (for legacy, that would still allow generic constraint on blittable) and explicit (for code that can be ugraded today easily)? On a struct that doesn't have a blittable attribute, the generic constraint blittable would check at least that the struct is transitively blittable compatible at least when it was compiled... |
What is the cost delta of requiring the keyword and pure run-time detection? |
As I see it, the keyword would only be used at compile time (both for struct and generic constraint) but I don't expect it to be used at runtime, as the runtime has to check anyway invalid usages of fixed (e.g via IL patching). The check at runtime is relatively small, blittable is computed once for a type, and checked when compiling a method that requires to "fixed" a T |
Assuming the cost is as minimal as @Xoof suggests, then I support the idea to back-port the feature via detection. My only question is: how does the compiler detect if a type is |
The problem with implicit detection is that it basically renders this feature useless. The purpose of this feature is to take what is today an implicit, fragile type constraint and turn it into an explicit, reliable one. Allowing implicit detection to mix in here breaks the idea of having an expilicit declarative model. The other issue with implicit detection is the unfortunate realities of our reference assembly generation tools. Consider all of the reference assemblies produced by assemblies from CoreFX. The policy there is to remove all private struct fields (a terrible idea for compilers, but a reality that must be considered). That means scores of structs which should never under any circumstances be considered blittable would be marked so in implicit detection. |
... and that was my concern (sorry for not articulating as nicely as you did, but yes the "reference assemblies" worried me). Thanks @jaredpar. Finally - with .Net Core and the "death of reflection" (kill it, kill it with fire! 🔥), is it even possible to know what a private field is reliably? |
Not entirely, it would make it half-useless 😅 (we would still benefit from having the generic constraint working at compile time). But I agree that it is not great in terms of language coherency...
Just to understand, what is the deal currently with them, could you elaborate? |
@xoofx. Imagine you have the following: class C
{
private int _value;
public int Value
{
get
{
return _value;
}
}
}
struct S
{
private C _c;
public C c
{
get
{
return _c;
}
}
} In the implementation assembly, However, it is fairly common to have 'reference' assemblies. For reference assemblies, all private members are stripped (generally), so we end up with: class C
{
public int Value
{
get
{
throw new NotImplementedException();
}
}
}
struct S
{
public C c
{
get
{
throw new NotImplementedException();
}
}
} Now, |
@tannergooding han, thanks for the precision, never looked exactly how much the ref assemblies kept around the original internal meta... so... "fixing" the reference assembly generation tools could actually make the implicit proposal "half-possible"? 😁 kidding, let's keep it explicit 👍 |
|
||
The primary drawback of this feature is that it serves a small number of developers: typically low level library authors or frameworks. Hence it's spending precious language time for a small number of developers. | ||
|
||
Yet these frameworks are are often the basis for the majority of .NET applications out there. Hence performance / correctness wins at this level can have a riple effect on the .NET ecosystem. This makes the feature worth considering even with the limited audience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
frameworks are are
Just a small nitpick from someone who knows basically nothing about what is said in the proposal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're at it, riple effect
should be ripple effect
.
As I mentioned here, please consider adding an IsBlittable getter to both, the Type and ValueType classes. |
} | ||
``` | ||
|
||
Such routines are advantageous because they are provably safe at compile time and allocation free. Interop authors today can not due this (even though it's at a layer where perf is critical). Instead they need to rely on allocating routines that have expensive runtime checks to verify values are correctly blittable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"due" -> "do"
``` c# | ||
Span<byte> Convert<T>(ref T value) where T : blittable { | ||
... | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From recent chat in LDM, the question came up: is ValueTuple<int, int>
blittable? (I assume it should) How would we achieve that?
One more note: the ValueTuple types are implemented with [StructLayout(LayoutKind.Auto)]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think ValueTuple<int, int>
should be blittable. This feature is for low-level interop. Limiting use of abstractions in low-level interop is fine. For example, generic PInvokes are not allowed and I do not think that it was ever raised as problem.
ValueTuple types are implemented with [StructLayout(LayoutKind.Auto)]
That makes them non-blittable by definition. This optimization would need to be undone for ValueTuple<int, int>
to be blittable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a large number of generic types, such as ValueTuple
, they are designed to be used with more than just blittable types, and there is no real way to make it 'blittable' without breaking back-compat (under the current proposal).
With the current proposal, structs will need to be explicitly marked as blittable (blittable struct
). If they are missing this decoration then they will not be considered blittable.
There are already limitation around using generics with any kind of P/Invoke. However, if we had a blittable
constraint, there is really no reason why this support could not be added (just like there is no reason to block taking the address of a generic type with blittable constraints on each typeparam).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
limitation around using generics with any kind of P/Invoke
no reason to block taking the address of a generic type with blittable constraints
These two are pretty different:
Taking address of generic type is simple, well-defined operation. You can do it today using System.Runtime.CompilerServices.Unsafe
. It is just a C# language limitation that you cannot write it directly.
Generics in P/Invokes is not simple and well defined. It is limitation of the PInvoke marshalling baked into the runtime. PInvoke marshalling rules are pretty complex. Throwing generics into the mix would make them even more complex.
Is this proposal even under active work? It's scheduled for 7.2 yet no progress is being made. Do you think it will be delayed to 7.X or abandoned altogether? |
@ghord i just forgot to merge the PR 😦
It is not scheduled for 7.2. The notations on this repo around milestones are more aspirations / suggestions to the compiler team. Essentially "it would be fine if this made it for 7.2". It is currently being considered for 7.3 by the compiler team. |
It was removed from Language Feature Status completely recently, plans changed? |
@tomrus88 Work has not started on that feature, but we're still planning to work on it in 7.3. |
@jcouv |
The language feature status page is meant to be status on actual changes happening in the compiler at the time. Essentially work that is scheduled / in progress that has resources allocated to it. It's not a commitment, can always decided to delay / cancel work. But it does try to add transparency to what the compiler team is working on. In this case we had scheduled the blittable work and then as we got closer to the end of C# 7.2 we decided it simply didn't fit and we backed off. This was due to a combination of design issues and time constraints.
For blittable we have devs assigned to it: @OmarTawfik. It's planned for C# 7.3 but as with all features that's subject to change for any number of reasons 😄
Static delegates aer trickier because they have runtime implications that we need to work out. Still trying to figure that out. |
## Unresolved questions | ||
[unresolved]: #unresolved-questions | ||
|
||
- blittable vs. unmanaged. The F# language has a very [similar feature](https://docs.microsoft.com/en-us/dotnet/articles/fsharp/language-reference/generics/constraints) which uses the keyword unmanaged. The blittable name comes from the use in Midori. May want to look to precedence here and use unmanaged instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"blittable type" has an existing definition in CLR interop already: https://docs.microsoft.com/en-us/dotnet/framework/interop/blittable-and-non-blittable-types . The existing definition is different from your definition, e.g. char
and bool
are not blittable in the existing definition. It may be a good idea to call it something else to avoid confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably worthwhile to keep this as blittable and remove char
and bool
from the definition.
That being said, bool
and char
are special in that they are considered "sometimes" blittable and, last I checked, the (CoreCLR) marshaller will still blit bool
and char
when the appropriate conditions are met (just like it will pin and directly reference a string
when the right conditions are met).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing definition also have "The following complex types are also blittable types: One-dimensional arrays of blittable types". What would you do about that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are going to go with the term "unmanaged type". It is already in use in the C# spec and F#.
|
||
A declarative, explicit opt in model makes it easier to code in this area. Developers can be very explicit about the types they intend for interop. This gives them compiler help when mistakes are made in their application and in any libraries they consume. | ||
|
||
The lack of compile time support also makes it difficult to abstract over blittable types. It's not possible for instance to author common helper routines using generic code: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is possible using System.Runtime.CompilerServices.Unsafe
package.
This is the initial proposal for the blittable feature as tracked by #187.