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!

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.

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;
}

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).

@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.

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.

@stephentoub Thank you very much.

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

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!