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

Replace use of Marshal.SizeOf with sizeof for blittable structs #3218

Open
vatsan-madhavan opened this issue Jul 3, 2020 · 5 comments
Open
Labels
Enhancement Requested Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@vatsan-madhavan
Copy link
Member

dotnet/pinvoke#463 (comment)

Marshal.SizeOf returns the size of the unmanaged view of the struct. sizeof returns the size of the managed view of the struct.

Unmanaged and managed views are identical for the blittable structs. It is better to use sizeof instead of Marshal.SizeOf for blittable structs. They return the same number and sizeof is much cheaper.

There are many uses of Marshal.SizeOf in the codebase - many of them likely for blittable types - that could be identified and updated to use faster sizeof.

/cc @dotnet/dotnet-winforms - This probably applies to Winforms as well.

@RussKie
Copy link
Member

RussKie commented Jul 6, 2020

Thank you @vatsan-madhavan, we're aware of this, and we've been making these changes as we go through our codebase.

@fabiant3 fabiant3 added the Enhancement Requested Product code improvement that does NOT require public API changes/additions label Jul 6, 2020
@fabiant3 fabiant3 added this to the Future milestone Jul 6, 2020
@weltkante
Copy link

weltkante commented Jul 13, 2020

Note that WinForms has had regressions when switching from Marshal.SizeOf to sizeof which you should be aware of.

In particular bool is not blittable but the runtime allows it anways (by design, has been enabled specifically by a PR, can't find it right now though linked below). So when switching from Marshal.SizeOf to sizeof you must make sure no bool is part of the struct. WinForms has various helper structs for differently sized booleans in the Windows API.

code from linked comment detailing the difference
  • Marshal.SizeOf for classic marshalling size
  • Marshal.OffsetOf for classic marshalling offsets
  • sizeof keyword for blitting size
  • pointer arithmetic for blitting offsets

Example where it makes a difference:

public struct Sample
{
    public int a;
    public bool b;
    public short c;
}

var sample = new Sample();
var blittingSize = sizeof(Sample);                              // 8
var blittingOffset = (byte*)&sample.c - (byte*)&sample;         // 6
var marshalSize = Marshal.SizeOf<Sample>();                     // 12
var marshalOffset = Marshal.OffsetOf<Sample>(nameof(Sample.c)); // 8

@vatsan-madhavan
Copy link
Member Author

Makes sense.

In practice correct use of bool would look like BOOL with a MarshalAs attribute anyway, which wouldn't have been a blittable struct in the first place.

@vatsan-madhavan
Copy link
Member Author

BTW https://docs.microsoft.com/en-us/dotnet/framework/interop/blittable-and-non-blittable-types calls out bool and char as non-blittable.

I believe they both work with sizeof (probably because they are in fact blittable in a strict sense) but in practice aren't blittable (because of BOOL/MarshalAs[UnmanagedType.Bool] and unicode/ansi auto-marshaling considerations respectively).

bool fields shouldn't appear in P/Invoke structs anyway - at least not without a MarshalAs attribute (or a comment indicating why the attribute is left out). Anything else is suspicious IMO. In general, structs with bool should indeed be treated presumptively as non-blittable, until it can be proven with further analysis that they are blittable.

Very similar arguments apply to structs with char fields as well.

@weltkante
Copy link

weltkante commented Jul 13, 2020

Yes bool/char is non-blittable but works anyways, it was specifically enabled here: dotnet/runtime#13772 (issue) and dotnet/runtime#1866 (PR). The runtime uses different size/alignment for bool than the classic marshalling layer, its just something to keep in mind when porting code to this syntax, besides that its well-defined and supported. Using helper structs is the right way to keep things readable considering the Windows API knows at least three different bool types, each with different sizes/rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Requested Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants