Biohazrd icon indicating copy to clipboard operation
Biohazrd copied to clipboard

Eliminate the use of MarshalAs for bools

Open PathogenDavid opened this issue 2 years ago • 2 comments

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 avatar Feb 12 '22 22:02 PathogenDavid

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 avatar Feb 12 '22 23:02 PathogenDavid

MarshalAs for return values and parameters has been eliminated as part of https://github.com/MochiLibraries/Biohazrd/issues/233 and https://github.com/MochiLibraries/Biohazrd/issues/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 avatar Feb 15 '22 00:02 PathogenDavid