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

Eliminate the use of MarshalAs for bools #237

Open
PathogenDavid opened this issue Feb 12, 2022 · 2 comments
Open

Eliminate the use of MarshalAs for bools #237

PathogenDavid opened this issue Feb 12, 2022 · 2 comments
Labels
Area-OutputGeneration Issues concerning the process of generating output from Biohazrd Concept-OutputFriendliness Issues concerning the friendliness of using the Biohazrd output Concept-OutputPerformance Issues concerning the performance of libraries generated using Biohazrd

Comments

@PathogenDavid
Copy link
Member

Right now we use MarshalAs for bool where possible and NativeBoolean where not.

We should instead prefer using trampolines where possible and NativeBoolean where not.

@PathogenDavid PathogenDavid added Area-OutputGeneration Issues concerning the process of generating output from Biohazrd Concept-OutputPerformance Issues concerning the performance of libraries generated using Biohazrd Concept-OutputFriendliness Issues concerning the friendliness of using the Biohazrd output labels Feb 12, 2022
@PathogenDavid
Copy link
Member Author

Oddly enough, using MarshalAs(I1) for bools doesn't trigger MarshalDirectiveException even when marshaling is disabled. Not sure if that's intentional but I'd still like to eliminate it either way.

PathogenDavid added a commit that referenced this issue Feb 15, 2022
@PathogenDavid
Copy link
Member Author

MarshalAs for return values and parameters has been eliminated as part of #233 and #236

While working on these, it dawned on me that emitting MarshalAs(I1) on fields may not be the worst thing ever.

As far as I can think, the marshaler only touches the fields of a struct for marshaling purposes when that struct is passed (directly or indirectly) by value or it's passed as a C# byref.

After the work on the above issues is completed, the latter situation won't ever happen in Biohazrd-generated code. The by-value case can, but only for structs which are not implicitly passed by reference. The rules vary between ABIs, but generally structs passed by value are only "truly" passed by value (from an ABI perspective not a programming language perspective) when they can fit within registers. Generally speaking this only happens if the struct is relatively small. If the struct is relatively small the performance hit of having the marshaler clone and manipulate the struct is also relatively small.

It's obviously still not ideal, but maybe the API clarity is worth it over using NativeBoolean everywhere? Still need to think on it.

One thing we could do is write a transformation which looks through all parameter/return types and checks if they're by-value and if they directly or indirectly reference a record which contains fields which use bools and opt-in to NativeBoolean for them (and continue using MarshalAs for everything else just in case.)

PathogenDavid added a commit that referenced this issue Mar 9, 2022
…y trampoline methods.

This commit also adds partial support for .NET 7 / C# 11.

Closes #236
Closes #79
Fixes #200
Contributes to #233
Contributes to #237
Workaround for #239
PathogenDavid added a commit that referenced this issue Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-OutputGeneration Issues concerning the process of generating output from Biohazrd Concept-OutputFriendliness Issues concerning the friendliness of using the Biohazrd output Concept-OutputPerformance Issues concerning the performance of libraries generated using Biohazrd
Projects
None yet
Development

No branches or pull requests

1 participant