Use generic Marshal.PtrToStructure

[ThomasGoulet73]

Description

Use generic Marshal.PtrToStructure instead of the non-generic. The generic one is slightly faster than the non-generic. Also, I removed the customs UnsafeNativeMethods.PtrToStructure which justs forwards to Marshal.PtrToStructure because it causes linker warnings because Marshal.PtrToStructure is annotated while UnsafeNativeMethods.PtrToStructure isn't.

Contributes to #3811

Customer Impact

None, same behavior as the non-generic.

Regression

No

Testing

I manually tested a few of the generic and non-generic and they gave the same result.

Risk

None, same behavior as the non-generic.

[ThomasGoulet73]

Hey @stephentoub, could I have your opinion here please?

In one of your PR, you changed Marshal.PtrToStructure for this unsafe code: https://github.com/dotnet/wpf/pull/4754/files#diff-30ce94d35c72b94385aa439cafead5bd0677f7b4037f0fefcfc7a03d3730a681L4702-R4705

What is best, the unsafe code or generic Marshal.PtrToStructure ?

If it's the unsafe code, when should we use it ? Should I change my PR to convert Marshal.PtrToStructure to this unsafe code ?

Thank you!

[stephentoub]

Both are equally "unsafe", so it's purely a performance / functionality discussion. If the type isn't blittable, use Marshal.PtrToStructure, as it appropriately handles all the necessary marshaling. If the type is blittable, it's way cheaper to just cast rather than using PtrToStructure.

Method	Mean	Allocated
Cast	0.0477 ns	-
PtrToStructureGeneric	26.2864 ns	24 B
PtrToStructureNonGeneric	28.2225 ns	24 B

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Runtime.InteropServices;

[MemoryDiagnoser]
public class Program
{
    public static void Main(string[] args) => BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);

    private IntPtr _ptr;

    [GlobalSetup]
    public unsafe void Setup() => _ptr = Marshal.AllocHGlobal(sizeof(MyPoint));

    [GlobalCleanup]
    public void Cleanup() => Marshal.FreeHGlobal(_ptr);

    [Benchmark]
    public unsafe MyPoint Cast() => *(MyPoint*)_ptr;

    [Benchmark]
    public MyPoint PtrToStructureGeneric() => Marshal.PtrToStructure<MyPoint>(_ptr);

    [Benchmark]
    public MyPoint PtrToStructureNonGeneric() => (MyPoint)Marshal.PtrToStructure(_ptr, typeof(MyPoint));
}

[StructLayout(LayoutKind.Sequential)]
public struct MyPoint
{
    public int X;
    public int Y;
}

[ThomasGoulet73]

Thank you!

Both are equally "unsafe", so it's purely a performance / functionality discussion

I didn't mean "unsafe" as "this code is less safe than this one", I just didn't know how to name your code and I used "unsafe" because it used unsafe blocks.

I'll revert this change and keep the usage of generic PtrToStructure everywhere else in this PR since the generic version is slightly faster than the non-generic.

I'll open an issue to use your code to cast IntPtr to blittable structs (Maybe also add a shared method to avoid having unsafe blocks everywhere to improve readability).

[lindexi]

@stephentoub Can I ask you why the PtrToStructureGeneric is faster than PtrToStructureNonGeneric?

The main different codes are as follows:

        public static object? PtrToStructure(IntPtr ptr, Type structureType)
        {
            // Ignore some code ...
            object structure = Activator.CreateInstance(structureType, nonPublic: true)!;
            PtrToStructureHelper(ptr, structure, allowValueClasses: true);
            return structure;
        }

        public static T? PtrToStructure<T>(IntPtr ptr)
        {
            // Ignore some code ...
            object structure = Activator.CreateInstance(structureType, nonPublic: true)!;
            PtrToStructureHelper(ptr, structure, allowValueClasses: true);
            return (T)structure;
        }

See:

https://github.com/dotnet/runtime/blob/52c6dcb45cc1e8e98d267c07f10fe42d89c71656/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs#L575-L600

https://github.com/dotnet/runtime/blob/52c6dcb45cc1e8e98d267c07f10fe42d89c71656/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs#L615-L634

Thank you.

[stephentoub]

For the generic, and a value type, the JIT generates a specialization of the method for that value type, such that the PtrToStructure method knows what the type is, which means it can do at compile time things the non-generic needs to do at runtime.

[lindexi]

@stephentoub Thank you very much.

[RussKie]

In general you want to avoid using Marshal class, and replace this with blittable structs and pointers

[ThomasGoulet73]

Yes, this is definitely something that I'm interested in doing in a separate PR. I would still keep this PR only on using the generic version since it's easier to review (Handles blittable and non-blittable), slightly faster than the non-generic overload and trim-friendly (Removes UnsafeNativeMethods.PtrToStructure which is not annotated for the linker).

Thank you!