dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

[API Proposal]: `RuntimeFieldHandle.Offset` #94976

Open MichalPetryka opened 9 months ago

MichalPetryka commented 9 months ago

Background and motivation

In some interop or unsafe manipulation scenarios being able to obtain field offset of a certain field without having to do ref arithmetics. As such, exposing such offset on RuntimeFieldHandle seems like it would allow programmers to access it in a cheap way without writing code that can break depending on JIT optimizations.

The API would return a sentinel value of -1 for static fields and fields added by EnC to avoid exceptions in high performance code.

Together with #94975 this can supersede #93946.

API Proposal

namespace System;

public struct RuntimeFieldHandle
{
    public int Offset { get; }
}

API Usage

struct S
{
    public int a;
    public float b;
    public byte c;
    public double d;
}

Console.WriteLine(typeof(S).GetField("d").FieldHandle.Offset); // prints 16

Alternative Designs

93946

Risks

This could make invalid code based on Offsets more common, same as #93946. This could confuse the runtimes and introduce unexpected crashes.

ghost commented 9 months ago

Tagging subscribers to this area: @dotnet/area-system-runtime See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation In some interop or unsafe manipulation scenarios being able to obtain field offset of a certain field without having to do ref arithmetics. As such, exposing such offset on `RuntimeFieldHandle` seems like it would allow programmers to access it in a cheap way without writing code that can break depending on JIT optimizations. The API would return a sentinel value of `-1` for static fields and fields added by EnC to avoid exceptions in high performance code. Together with #94975 this can supersede #93946. ### API Proposal ```csharp namespace System; public struct RuntimeFieldHandle { public int Offset { get; } } ``` ### API Usage ```csharp struct S { public int a; public float b; public byte c; public double d; } Console.WriteLine(typeof(S).GetField("d").FieldHandle.Offset); // prints 16 ``` ### Alternative Designs #93946 ### Risks This could make invalid code based on Offsets more common, same as #93946. This could confuse the runtimes and introduce unexpected crashes.
Author: MichalPetryka
Assignees: -
Labels: `api-suggestion`, `area-System.Runtime`, `untriaged`
Milestone: -
MichalStrehovsky commented 9 months ago

Together with https://github.com/dotnet/runtime/issues/94975 this can supersede https://github.com/dotnet/runtime/pull/93946.

Just a note that those are 100% non-reflection codepaths. This is pure reflection - might as well put this on FieldInfo.

MichalPetryka commented 9 months ago

Just a note that those are 100% non-reflection codepaths. This is pure reflection

The runtime could fold accesses to Offset with ldtoken/proposed UnsafeAccessor so it could become "free reflection" like things like typeof(T).IsValueType are.

might as well put this on FieldInfo

I have three reasons why I think RuntimeFieldHandle is a better place:

  1. doing it on FieldInfo would require allocating a new instance to access it from handle and would make folding like I've described earlier harder.
  2. this is a lower level, runtime specific, concept so I feel like it's more appropriate to expose it here, just like GetFunctionPointer() is exposed on RuntimeMethodHandle, not MethodBase
  3. this property wouldn't make sense for custom, non runtime based FieldInfos and RuntimeFieldHandle is restricted to those.
MichalStrehovsky commented 9 months ago

The runtime could fold accesses to Offset with ldtoken/proposed UnsafeAccessor so it could become "free reflection" like things like typeof(T).IsValueType are.

This would only get expanded if the JIT feels like it. It would probably not be expanded in Tier-0 for example. For Native AOT, the whole program analysis that we do before codegen cannot assume any particular codegen optimization so the whole program view will have "this is reflecting over a field". It's hard to clean that up afterwards both in terms of the field being visible from reflection, and in terms of the reflection infrastructure code to access fields.

MichalPetryka commented 9 months ago

Would #83021 solve this issue?

MichalStrehovsky commented 9 months ago

Would #83021 solve this issue?

It would solve the native AOT part of the problem, yes (provided the JIT expands it). It's not planned for .NET 9.

In general, I still don't see how this is better than #93946. This requires a lot of things to happen.

yushroom commented 7 months ago

@MichalStrehovsky from discussion https://github.com/dotnet/runtime/discussions/97203 I'm a Unity game developer. I want to use NativeAOT to replace IL2cpp to improve performance. Unity engine runtime (both Mono and IL2CPP) relies heavily on field offset to access member of object. This proposal is really helpful.

MichalStrehovsky commented 7 months ago

@yushroom would it be possible to add links to Unity docs about this?

I want to understand why you need managed offsets, as opposed to the native offsets (available using the existing Marshal.OffsetOf API). I would expect interop scenarios (such as interop with the Unity engine) to use interop APIs.

hamarb123 commented 6 months ago

I'm trying to implement Volatile.Read/Write/etc<T> for general T (non-atomic). To do this, I need to either be able to manually emit the applicable barrier (which I cannot do), or I need to be able to determine whether parts of a managed struct T are object or not.

It seems impossible to do on NAOT currently. On normal runtime I can get the size of a type with ILGen calls to sizeof and determine the field offset with ldflda and sub quite trivially (which would obviously be nice to be able to avoid) - this allows me to figure out whether the first and last sizeof(nuint) bytes are an object or not. The problem on NativeAOT is two-fold: firstly, I may have managed struct fields on my managed struct, so I need to be able to get the size from a Type, and secondly there is no way to get the field offsets, so I cannot see which parts of memory are managed.

I tried other ideas first (since I recall running into this issue before) of just boxing it and using reflection to set stuff and reading out all the parts which were managed (by only writing to either managed or unmanaged, and then comparing to 0), but the issues I faced here were: can't do it on inline arrays at all, padding (automatic or additional) cannot be written to with reflection so I cannot detect where it is, I cannot construct an object of any general type (therefore I cannot set it to the fields), and Marshal.OffsetOf won't even remotely do what I want.

The IL instructions, for which there are no built-in APIs, also don't work properly in a number of cases: https://github.com/dotnet/runtime/issues/91530, but I feel like these APIs would be useful even if I didn't have the issue with implementing Volatile APIs that should work.

jkotas commented 6 months ago

these APIs would be useful even if I didn't have the issue with implementing Volatile APIs that should work.

I do not see how these APIs can help you to do this efficiently and reliably with native AOT. If you are running into issues with volatile. prefix handling by the JIT, it would be best to fix the issue in the JIT. (Also, I expect that it would be much simpler fix than the work required to build the APIs proposed here.)

hamarb123 commented 6 months ago

I do not see how these APIs can help you to do this efficiently and reliably with native AOT.

Basically, I can implement them like this:

Write:

Read:

It's not perfect (it definitely could be faster - since the JIT doesn't combine all of the ops perfectly), but it's not too bad either. The problem on NativeAOT is that there is no way to determine for managed types whether those first & last alignof(T)s are managed or unmanaged.

If you are running into issues with volatile. prefix handling by the JIT, it would be best to fix the issue in the JIT. (Also, I expect that it would be much simpler fix than the work required to build the APIs proposed here.)

I'm happy for that to be done :) I just don't know how to do it - I would rather just use those, but for now that's not really an option.