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

StructLayouts that differ between architectures and/or operating systems #60573

Open
DaZombieKiller opened this issue Oct 18, 2021 · 15 comments
Open

Comments

@DaZombieKiller
Copy link
Contributor

Today, if you need to wrap a native API that has differing structure layouts (namely, structure packing) between platforms, you're stuck in a very messy scenario with not many good options to proceed. An example of such a scenario is Valve's Steamworks API, which has a series of callback structures with packing that varies per platform (8 on Windows, 4 everywhere else).

So, when faced with this problem, some solutions that come to mind are:

  • Ship per-RID assemblies
    • Not realistically feasible outside of the runtime.
  • Duplicate the structures per platform
    • This requires duplication of function signatures that use these structures by value.
    • It also requires duplication of any other structures that use these structures.
  • Use interfaces
    • Results in boxing, which may or may not be acceptable depending on your scenario.
    • Boxing can be avoided by using generics, but this results in vastly more complicated bindings.

None of these are particularly appetizing, and all of them have significant drawbacks. Ideally, the runtime could provide some method to annotate structures to allow their layout to change depending on the current platform. If we ignore everything except structure packing, such a feature could look like:

[StructLayout(LayoutKind.Sequential, Pack = 4)]
[StructPackOverride(PlatformID.Win32NT, Pack = 8)]
public struct SomeCallbackStructure {}

Which would define a struct with a packing value of 8 on Windows, and 4 everywhere else.

cc @jkoritzinsky @AaronRobinsonMSFT

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Oct 18, 2021
@AaronRobinsonMSFT
Copy link
Member

This seems like a great opportunity for customization in our struct marshalling source generator that @jkoritzinsky is working on. I'm hesitant to change any of our built-in marshalling support to enable this sort of functionality.

/cc @elinor-fung

@DaZombieKiller
Copy link
Contributor Author

I'm hesitant to change any of our built-in marshalling support to enable this sort of functionality.

I should mention that for my specific use cases I am working purely with blittable structures, avoiding marshaling where I can. So ideally this would affect both the managed and unmanaged layouts.

@AaronRobinsonMSFT
Copy link
Member

@DaZombieKiller Let me elaborate on my primary concern about built-in support. The Pack details are apart of the metadata – StructLayoutAttribute is a pseudo-attribute. This means it is sort of ugly to have something clearly declared in metadata but have another attribute that overrides that setting. It makes respecting this new attribute cumbersome since now metadata must be confirmed to be "correct" since the user could override that.

In order to improve this we would likely need some sentinel value to indicate – look somewhere else for the value. That isn't impossible but would require thought and coordinate with the Roslyn team.

@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Oct 18, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 7.0.0 milestone Oct 18, 2021
@DaZombieKiller
Copy link
Contributor Author

That makes a lot of sense, yeah. The example in the OP was more of a "this is what a potential solution to this could look like", moreso than a concrete proposal. If supporting differing layouts in the runtime itself proves to be too much of a headache, having APIs akin to StructureToPtr/PtrToStructure (which I imagine would be required by the generator if things were to go that route) would certainly still be enough to make this scenario far less of a headache to deal with.

@tannergooding
Copy link
Member

Its likely also worth considering in what scenarios this occurs. If such APIs appear as part of particularly "hot calls", then it may be problematic to require marshalling to occur. It can also impact where changes become visible if pointers, refs, or arrays are involved.

I'd expect that work in this area might not impact interop at all (at least directly), but would by relegated to the class layout logic in the VM. Noting that touching the class layout logic is likely just as concerning overall since touching that logic has historically been semi-problematic 😄

Much of this could be simpler if it was more trivial to support per-RID assemblies in .NET (although even that can have problems due to doubling size of things generally for a 1-or-2 types that need the customization).

@jkotas
Copy link
Member

jkotas commented Oct 18, 2021

[StructPackOverride(PlatformID.Win32NT, Pack = 8)]

Note that the conditions that express the platform differences can be quite complex. For example "Windows and not ARM" here:

#if (WIN32 && !ARCH_arm)
[StructLayoutAttribute(LayoutKind.Sequential, Pack = 2)]
#else
[StructLayout(LayoutKind.Sequential, Pack = 8)]
#endif
. I think it would be hard to make the declarative scheme like StructPackOverride to be general purpose enough.

@DaZombieKiller
Copy link
Contributor Author

Note that the conditions that express the platform differences can be quite complex.

This is a good point, and it got me thinking about potential alternatives. One that came to mind was something along the lines of:

public interface IRuntimeStructLayout
{
    static abstract int Pack { get; }
    static abstract int Size { get; }
    static abstract CharSet CharSet { get; }
}

public sealed class RuntimeStructLayoutAttribute<T> : Attribute
    where T : IRuntimeStructLayout
{
    public int Pack => T.Pack;
    public int Size => T.Size;
    public CharSet CharSet => T.CharSet;
}

Which would be used like this:

[RuntimeStructLayout<SomeCallbackLayout>]
public struct SomeCallback
{
}

sealed class SomeCallbackLayout : IRuntimeStructLayout
{
    public static int Pack => OperatingSystem.IsWindows() ? 8 : 4;
    public static int Size => 0;
    public static CharSet CharSet => CharSet.None;
}

I think such an approach would be fine if it were to be used for marshalling, but it's overall an unsatisfying solution because the requirement of code execution to get the values feels like it would be unsuitable for the managed layout (and thus wouldn't affect access through ref, pointers, etc.)

@jkotas
Copy link
Member

jkotas commented Oct 20, 2021

This solution would be incompatible with AOT compilation. It is equivalent to emitting the type definitions at runtime using Reflection.Emit. If fact, you can implement a solution like this using Reflection.Emit today: move the types with dynamic layout into a separate assembly, do not include this assembly in the application and instead emit it using Reflection.Emit at runtime. It is not something that I would recommend.

@DaZombieKiller
Copy link
Contributor Author

Yeah, AOT was another concern I had. I suppose it could be theoretically possible if there were heavy restrictions placed on what the IRuntimeStructLayout methods could do, but it'd be a monumental (and likely non-worthwhile) effort that would be better served investigating other options. The Reflection.Emit trick sounds clever (I assume you'd compile against a ref assembly that would look similar to what you emit at runtime?), but definitely not something I would want to pursue in a production codebase.

@danmoseley
Copy link
Member

Ship per-RID assemblies
Not realistically feasible outside of the runtime.

Could you say more about this? Doesn't Nuget hide the complexity from the consumer here?

@tannergooding
Copy link
Member

@danmoseley, in many senses yes. However, NuGet has a number of tracked issues here that prevent the experience from working cohesively. Some of the ones I can remember off the top of my head are:

In particular the some of the issues I've hit are that:

  • While its decently trivial to create a package which contains per-TFM assemblies, there is no simple way to do the same for per-RID.
    • This applies to the SDK in general where, as far as I know, there isn't some way to compile for multiple RIDs automatically (and specifying a /p:RuntimeIdentifier at the solution level is actually blocked by today with an error).
  • If you do the manual work of creating per-RID assemblies, you are realistically forced to put them all into a single NuGet which can cause increased network bandwidth and other issues for the inner loop.
    • While many assemblies are small, supporting per RID assemblies for 10 OS/architecture combinations quickly blows up in terms of size and build time complexity
    • When the difference is 2 types out of say 20k (where one of my libraries is right now), this becomes a very bad edge case of forcing users to go a massively complex route for something which ideally could be specified in metadata
    • There is some support for runtime.json to split into per-RID nuget packages, but this is not officially supported for public use (the runtime itself uses it) and has various issues because of that
  • There is no support for generating shared reference assemblies today
    • You get a reference assembly per TFM/RID, even if the public surface area is all the same
  • There is no support for switching on "just" Operating System or "just" bitness, you must specify both in conjunction
    • Linux can be particularly bad where it can be per-distro in the worst case, such as with packaging native dependencies
    • Some of this recently improved with net60-windows and the like, but then you aren't considered "AnyCPU" compatible which forces downstream changes in your dependents and a more complex build process/setup for them as well

@DaZombieKiller
Copy link
Contributor Author

Tanner's comment effectively summarizes most of the issues I could think of. One of the largest concerns for my own use cases is that there is a massive amount of code duplication (there is a not-insignificant number of structs that need to have "runtime layout" here), so with per-RID assemblies a project will need to include each platform-specific assembly (assuming the application or library consuming the code isn't built for a specific RID).

@AraHaan
Copy link
Member

AraHaan commented Nov 7, 2021

I would also like if it was possible to have a single defined structure for when other things like so exist as well:

  • A structure with 4 members on x86, 5 on x64, 4 on ARM, 5 on ARM64.

As such I often would like to define a structure with all 5 members and use it with AnyCPU and would love to mark that 5th member as:

[StructMember(StructMemberKind.Hidden)]
[StructMemberOverride(TargetCpuType.X64, StructMemberKind.Public)]
[StructMemberOverride(TargetCpuType.ARM64, StructMemberKind.Public)]

Where the struct is for calling into native code and StructMemberKind then would control exposure of that member to sizeof() at runtime where if say, the .NET Runtime / process is running under x86 or ARM that member is not calculated with sizeof() nor is it passed into native code, effectively hiding it unless the same code is executing under x64 or ARM64.

Reasonings? Well this could benefit situations like what is noticed in TerraFX.Interop.Windows where one might want both the 32 bit and 64 bit structures to some of the Windows APIs where the only difference is an added member under x64 or even ARM64. With something like this it could improve the codegen inside of TerraFX.Interop.Windows and also make the end users even more happy without adding a burden to @tannergooding for supporting x86 users at the moment and not just dropping 32 bit support entirely.

@tannergooding
Copy link
Member

Reasonings? Well this could benefit situations like what is noticed in TerraFX.Interop.Windows where one might want both the 32 bit and 64 bit structures to some of the Windows APIs where the only difference is an added member under x64 or even ARM64. With something like this it could improve the codegen inside of TerraFX.Interop.Windows and also make the end users even more happy without adding a burden to @tannergooding for supporting x86 users at the moment and not just dropping 32 bit support entirely.

Could we please not make assumptions about people's libraries and what would or would not be beneficial.

It's fine to request a feature that you think would be beneficial. It is not fine to try and tie it into someone else's library without first checking with the author if it's even something impacting them. There are limitations to what can and cannot be supported here and structs having an additional member between 32-bit and 64-bit is something I've hit less than 10 times in 30k types. The different packing issue is likewise rare overall, but it is at least quite a bit more common across various libraries/ecosystems.

Neither of these issues would actually need runtime support if there per-RID building, packing, and distributing was simpler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

7 participants