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

Implement infra to support marshalling blittable generics (equivalent to .NET 5 support) #80

Merged

Conversation

jkoritzinsky
Copy link
Member

Introduce new attributes to support marshalling blittable generic types with generic field types. The user can apply the [BlittableTypeIfGenericParametersBlittable] attribute to a type with generic field types, and apply the [ContributesToBlittability] attribute to any generic parameters that are used in fields. When the type is instantiated with blittable types in the type parameters marked with [ContributesToBlittability] and the instance fields that have non-generic types are all blittable, the type marked with [BlittableTypeIfGenericParametersBlittable] is considered blittable.

This enables developers to use their generic types that they are able to use in the built-in marshalling with the source-generated marshalling system by adding a few attributes.

…eric type parameters are blittable. Enables support for equivalent behavior to the .NET 5.0 support for marshalling generic blittable types.
@jkoritzinsky jkoritzinsky added the area-DllImportGenerator Source Generated stubs for P/Invokes in C# label Sep 2, 2020
@AaronRobinsonMSFT
Copy link
Member

@jkoritzinsky I don't understand why this is needed at present. The blittability of a type should be commutable because we need fully instantiated types - if there is an interesting and practical case where that isn't true I would like to hear about it.

Basically, what are we gaining with this right now?

@jkoritzinsky
Copy link
Member Author

jkoritzinsky commented Sep 2, 2020

The goal of this work is to enable handling blittability in generic types without the consumer looking at the private fields of the type. For example, given the following structure, the current system (without this PR) has no way to determine if the type is blittable without violating the constraint that we don't look at private implementation details on the consuming side:

struct Foo<T>
{
    T field;
}

We can't mark Foo<T> as blittable because it isn't always blittable. We also can't use the NativeMarshalling attribute to specify marshalling if it's supposed to marshal as though it was blittable since the "native type" specified in the NativeMarshalling attribute has to be blittable.

The only way to marshal this would be to define a native type (losing the perf advantages of blittability), and specify the [MarshalUsing] attribute at every usage of the type with the instantiated native type, which is very tedious and honestly would be an eyesore if the type is used in a lot of interop calls.

@jkoritzinsky
Copy link
Member Author

With this PR, we could mark Foo<T> as blittable when T is blittable. At usages of Foo<T> in interop scenarios, it would be marshalled with the blittable perf boosts when it's blittable, and would be illegal to marshal (without a [MarshalUsing], same as every type that isn't marked with any interop attributes) when T is not blittable. So for example, Foo<double> would be marshallable as blittable with this work, but not today.

@jkotas
Copy link
Member

jkotas commented Sep 2, 2020

without the consumer looking at the private fields of the type

Private fields are not really private when it comes to interop. C# determines whether the type is unmanaged by looking at private fields and we include the private fields in reference assemblies for this reason.

I think it would be ok to look at private fields of generic types to determine whether they are really blittable.

@AaronRobinsonMSFT
Copy link
Member

The goal of this work is to enable handling blittability in generic types without the consumer looking at the private fields of the type.

Agreed in the longer term. However, there is a lot here and still a lot in the document for struct support not implemented. My issue here is this is defining new logic and semantics for a scenario that we presently have no need to support for our primary goal which is support > 80 % of the BCL and SPCL. There are potential other solutions involving enriching existing attributes I believe or relying on other forms of detection. We should also consider those with trade offs and fully digest the scope of the current struct marshalling proposal already defined. We already have 4 new attributes and an entire semantic type layout that must be adhered to. Before going down the "solve all the problems" we should see how the existing semantics unfold in the process.

@jkoritzinsky
Copy link
Member Author

without the consumer looking at the private fields of the type

Private fields are not really private when it comes to interop. C# determines whether the type is unmanaged by looking at private fields and we include the private fields in reference assemblies for this reason.

I think it would be ok to look at private fields of generic types to determine whether they are really blittable.

We don't keep private fields in our reference assemblies. We keep some dummy private fields to ensure that a type is correctly considered unmanaged or managed, but that's it. We generate a private int field if the struct has no reference type fields, and a private object field if the type has reference type fields.

For example, if we were to just look at private fields, we'd accidentally allow marshalling a System.Nullable<T> for a blittable T.

In the reference assembly, System.Nullable<T> has fields of type T and type int, but in the implementation, System.Nullable<T> has fields of type T and type bool. If we look at private fields, then we'd allow marshalling a System.Nullable<double>, which isn't blittable once you look at the implementation available at runtime.

@jkotas
Copy link
Member

jkotas commented Sep 3, 2020

We keep some dummy private fields to ensure that a type is correctly considered unmanaged or managed, but that's it.

In particular, we do generate the fields of type T, so you can tell whether the struct has field of type T or not. It think it is all that's really required for this.

We generate a private int field if the struct has no reference type fields, and a private object field if the type has reference type fields.

Yes, and we depend on annotations matching between the ref assembly and implementation for situations like this, for non-generic cases too.

My issue here is this is defining new logic and semantics for a scenario that we presently have no need to support for our primary goal

I agree with this. I am not convinced that we need these attributes. Can it be just:

  • Generic types can be marked [BlittableType].
  • If a generic [BlittableType] type has a field of the generic argument type, the generic argument has to be blittale too. Otherwise, the source generator produces error.

No new attributes needed.

What are the cases that this would not work for?

@jkoritzinsky
Copy link
Member Author

@jkotas that model works pretty well. I've tested it locally and it covers this scenario. I'll push out an update that just enhances [BlittableType] as per your model instead of the model with new attributes.

Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
@jkoritzinsky jkoritzinsky merged commit 91b131e into dotnet:DllImportGenerator Sep 3, 2020
@jkoritzinsky jkoritzinsky deleted the generics-blittable branch September 3, 2020 22:58
jkoritzinsky added a commit to jkoritzinsky/runtime that referenced this pull request Sep 20, 2021
runtimelab-bot pushed a commit that referenced this pull request Feb 8, 2022
# Local heap optimizations on Arm64

1. When not required to zero the allocated space for local heap (for sizes up to 64 bytes) - do not emit zeroing sequence. Instead do stack probing and adjust stack pointer:

```diff
-            stp     xzr, xzr, [sp,#-16]!
-            stp     xzr, xzr, [sp,#-16]!
-            stp     xzr, xzr, [sp,#-16]!
-            stp     xzr, xzr, [sp,#-16]!
+            ldr     wzr, [sp],#-64
```

2. For sizes less than one `PAGE_SIZE` use `ldr wzr, [sp], #-amount` that does probing at `[sp]` and allocates the space at the same time. This saves one instruction for such local heap allocations:

```diff
-            ldr     wzr, [sp]
-            sub     sp, sp, #208
+            ldr     wzr, [sp],#-208
```

Use `ldp tmpReg, xzr, [sp], #-amount` when the offset not encodable by post-index variant of `ldr`:
```diff
-            ldr     wzr, [sp]
-            sub     sp, sp, #512
+            ldp     x0, xzr, [sp],#-512
```

3. Allow non-loop zeroing (i.e. unrolled sequence) for sizes up to 128 bytes (i.e. up to `LCLHEAP_UNROLL_LIMIT`). This frees up two internal integer registers for such cases:

```diff
-            mov     w11, #128
-                                               ;; bbWeight=0.50 PerfScore 0.25
-G_M44913_IG19:        ; gcrefRegs=00F9 {x0 x3 x4 x5 x6 x7}, byrefRegs=0000 {}, byref, isz
             stp     xzr, xzr, [sp,#-16]!
-            subs    x11, x11, #16
-            bne     G_M44913_IG19
+            stp     xzr, xzr, [sp,#-112]!
+            stp     xzr, xzr, [sp,#16]
+            stp     xzr, xzr, [sp,#32]
+            stp     xzr, xzr, [sp,#48]
+            stp     xzr, xzr, [sp,#64]
+            stp     xzr, xzr, [sp,#80]
+            stp     xzr, xzr, [sp,#96]
```

4. Do zeroing in ascending order of the effective address:

```diff
-            mov     w7, #96
-G_M49279_IG13:
             stp     xzr, xzr, [sp,#-16]!
-            subs    x7, x7, #16
-            bne     G_M49279_IG13
+            stp     xzr, xzr, [sp,#-80]!
+            stp     xzr, xzr, [sp,#16]
+            stp     xzr, xzr, [sp,#32]
+            stp     xzr, xzr, [sp,#48]
+            stp     xzr, xzr, [sp,#64]
```

In the example, the zeroing is done at `[initialSp-16], [initialSp-96], [initialSp-80], [initialSp-64], [initialSp-48], [initialSp-32]` addresses. The idea here is to allow a CPU to detect the sequential `memset` to `0` pattern and switch into write streaming mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-DllImportGenerator Source Generated stubs for P/Invokes in C#
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants