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

[API Proposal]: Get the managed size of a System.Type instance #97344

Closed
jkoritzinsky opened this issue Jan 22, 2024 · 12 comments · Fixed by #100618
Closed

[API Proposal]: Get the managed size of a System.Type instance #97344

jkoritzinsky opened this issue Jan 22, 2024 · 12 comments · Fixed by #100618
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@jkoritzinsky
Copy link
Member

Background and motivation

As part of the same problem as #97341, we have a scenario where we have a System.Type, we cannot rewrite the code in an AOT-safe manner to represent the type as a generic argument, and we want to be able to call Unsafe.SizeOf<T> for our System.Type instance.

This issue proposes a new method to get the size of the instance of a managed type from its RuntimeTypeHandle.

API Proposal

namespace System.Runtime.CompilerServices;

public class RuntimeHelpers
{
+    public static int SizeOf(RuntimeTypeHandle type);
}

API Usage

var abi_element_size = RuntimeHelpers.SizeOf(AbiType.TypeHandle);
var byte_length = length * abi_element_size;
m._array = Marshal.AllocCoTaskMem(byte_length);
m._marshalers = new object[length];
var element = (byte*)m._array.ToPointer();
for (int i = 0; i < length; i++)
{
    m._marshalers[i] = Marshaler<T>.CreateMarshaler(array[i]);
    Marshaler<T>.CopyAbi(m._marshalers[i], (IntPtr)element);
    element += abi_element_size;
}

Alternative Designs

We could place the API on System.Runtime.CompilerServices.Unsafe next to SizeOf<T>().

Risks

No response

@jkoritzinsky jkoritzinsky added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime labels Jan 22, 2024
@ghost
Copy link

ghost commented Jan 22, 2024

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

As part of the same problem as #97341, we have a scenario where we have a System.Type, we cannot rewrite the code in an AOT-safe manner to represent the type as a generic argument, and we want to be able to call Unsafe.SizeOf<T> for our System.Type instance.

This issue proposes a new method to get the size of the instance of a managed type from its RuntimeTypeHandle.

API Proposal

namespace System.Runtime.CompilerServices;

public class RuntimeHelpers
{
+    public static int SizeOf(RuntimeTypeHandle type);
}

API Usage

var abi_element_size = RuntimeHelpers.SizeOf(AbiType.TypeHandle);
var byte_length = length * abi_element_size;
m._array = Marshal.AllocCoTaskMem(byte_length);
m._marshalers = new object[length];
var element = (byte*)m._array.ToPointer();
for (int i = 0; i < length; i++)
{
    m._marshalers[i] = Marshaler<T>.CreateMarshaler(array[i]);
    Marshaler<T>.CopyAbi(m._marshalers[i], (IntPtr)element);
    element += abi_element_size;
}

Alternative Designs

We could place the API on System.Runtime.CompilerServices.Unsafe next to SizeOf<T>().

Risks

No response

Author: jkoritzinsky
Assignees: -
Labels:

api-suggestion, area-System.Runtime

Milestone: -

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 22, 2024
@AaronRobinsonMSFT
Copy link
Member

Related #24200

@MichalStrehovsky
Copy link
Member

Would this work for both reference types and value types? My proposal would be to either make this throw for non-valuetypes, or have it return IntPtr.Size, same as Unsafe.SizeOf<T>.

Size of classes is tricky (do we want the aligned or unaligned size?) and unless we have a use case to decide what size this is, it's better to just proactively block it. Nothing good would probably come out of it.

@jkoritzinsky
Copy link
Member Author

jkoritzinsky commented Jan 23, 2024

I honestly don't care about the experience for reference types. I only need this for value types. I think either having it throw or return IntPtr.Size is reasonable.

Since this is proposed as "sizeof(T) but with a dynamic T", I'd be inclined to return IntPtr.Size to match Unsafe.SizeOf<T>.

@EgorBo
Copy link
Member

EgorBo commented Feb 1, 2024

Should it be

namespace System.Runtime.CompilerServices
{
  public static class Unsafe
  {
    public static int SizeOf<T>();
+   public static int SizeOf(Type type);
    ...

instead? To improve API discoverability and I think we use this pattern for some other APIs, e.g. RuntimeHelpers.IsReferenceOrContainsReferences

@tannergooding
Copy link
Member

👍 to Egor's suggestion, having it as a neighboring overload is definitely more consistent with how we do it for other overloads.

Is there any particular reason why it needs to be RuntimeTypeHandle instead of simply Type?

@jkoritzinsky
Copy link
Member Author

I'm like using RuntimeTypeHandle when it needs to be a runtime-loaded type as opposed to a user-provided derived type.

I'm okay changing it if that's desirable.

@tannergooding
Copy link
Member

Are there any benefits or drawbacks to using one over the other here?

AFAIR, RuntimeTypeHandle is basically just a thin wrapper over RuntimeType and so I believe taking Type will just make it easier for users to use the API if they don't have a concrete type available.

@MichalPetryka
Copy link
Contributor

The JIT is able to fold all conversions away FYI:
image
image

@jkotas
Copy link
Member

jkotas commented Feb 1, 2024

Are there any benefits or drawbacks to using one over the other here?

If the API takes Type, it needs to validate whether the Type is RuntimeType first. This validation takes a few extra instructions.

Type is more usable. RuntimeTypeHandle is more oriented towards low-level perf oriented.

The JIT is able to fold all conversions away FYI

If you know the concrete type, it is better to use the existing generic SizeOf method. It is going to faster (or at least as fast) than this method.

@tannergooding tannergooding added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Feb 1, 2024
@tannergooding
Copy link
Member

Sounds like RuntimeTypeHandle is the right thing here and API review can discuss if we want it on RuntimeHelpers or Unsafe next to SizeOf, so I've marked this ready-for-review

@jkoritzinsky jkoritzinsky added this to the 9.0.0 milestone Feb 7, 2024
@bartonjs
Copy link
Member

bartonjs commented Apr 2, 2024

Video

  • Looks good as proposed
namespace System.Runtime.CompilerServices;

public partial class RuntimeHelpers
{
     public static int SizeOf(RuntimeTypeHandle type);
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 2, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Apr 3, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants