winforms icon indicating copy to clipboard operation
winforms copied to clipboard

Consider LibraryImport for PInvoke generation

Open kant2002 opened this issue 4 years ago • 10 comments

The LibraryImportGenerator experiment lands .NET 7 and provide slightly better performance with high compatibility bar. Also LibraryImportGenerator probably would be productized for .NET 7 https://github.com/dotnet/runtime/issues/60595

.NET Team itself embrace it in core .NET - See https://github.com/dotnet/runtime/issues?q=is%3Aissue+is%3Aopen+LibraryImport

Unfortunately COM marshalling does not supported, but given that ComWrappers should be done here, it's probably would not be a problem.

Open issues:

  • [x] https://github.com/dotnet/runtime/issues/68634

kant2002 avatar Nov 24 '21 10:11 kant2002

I think it should wait until it actually gets shipped and if it can add support for com marshalling then use it to avoid possibly breaking changes.

AraHaan avatar Apr 27 '22 07:04 AraHaan

I create issue for exploration. but I want to note, that Com Marshalling does not applicable. .NET 7 should work with NativeAOT/disabled built-in COM without bandaid required for previous versions.

Given that Preview 5 would have LibraryImportGenerator I think I can start playing with it here.

kant2002 avatar Apr 27 '22 14:04 kant2002

First work is started in https://github.com/dotnet/winforms/pull/7172

kant2002 avatar May 14 '22 11:05 kant2002

I think Winforms will still need a way to use COM then when this happens. Perhaps one should ask the maintainers of the generator to add COM marshaling support so that way there won't be any possible regressions in winforms.

AraHaan avatar May 14 '22 19:05 AraHaan

/cc: @AaronRobinsonMSFT for visibility

RussKie avatar May 15 '22 02:05 RussKie

I think Winforms will still need a way to use COM then when this happens.

WinForms can start to consume LibraryImport now and that would be much appreciated. We started doing this in https://github.com/dotnet/aspnetcore/pull/41573 and found some deficiencies so it is possible WinForms will also find issues we should address.

Perhaps one should ask the maintainers of the generator to add COM marshaling support so that way there won't be any possible regressions in winforms.

This is on deck - see https://github.com/dotnet/runtime/issues/66674.

/cc @dotnet/interop-contrib

AaronRobinsonMSFT avatar May 15 '22 02:05 AaronRobinsonMSFT

For the record LibraryImport work started https://github.com/dotnet/winforms/pull/7172

based on my previous attempts we need a lot of structure marshalers. What’s recommendations for dealing with structures? There a lot of them in WinForms and writing all of marshalers would be boring. Can simple marshalers be generated for us for this case?

kant2002 avatar May 15 '22 03:05 kant2002

Can simple marshalers be generated for us for this case?

That is something we have discussed. We are still looking for the justification of cost though. Knowing how many marshallers would need to be written manually in WinForms coupled with a few examples would be helpful in understanding the benefit.

AaronRobinsonMSFT avatar May 15 '22 04:05 AaronRobinsonMSFT

Okay. I wrote a small tool which analyze System.Windows.Forms.Primitives where most DllImport lives.

Methods analysis:

=== Total DllImport methods: 447 ===
=== Methods with unmanaged parameters : 252 ===
=== Print marshal with batteries methods (String,HandleRef) : 18 ===

=== Additional code required for methods below  ===

=== Print unclassified methods : 0 ===
=== Print string builder methods : 6 ===
=== Print class methods : 17 ===
=== Print com interface methods : 23 ===
=== Print managed struct methods : 10 ===
=== Print unmanaged struct methods : 137 ===

And types analysis

======== Type marshal reasons =========
ComInterface Interop+Ole32+IClassFactory2
ComInterface Interop+Ole32+ILockBytes
ComInterface System.Runtime.InteropServices.ComTypes.IDataObject
ComInterface Interop+Ole32+IDropTarget
ComInterface Interop+Ole32+IStorage
ComInterface System.Runtime.InteropServices.ComTypes.ITypeLib
ComInterface Interop+Ole32+IFont
ComInterface Interop+Ole32+IFontDisp
ComInterface Interop+Oleaut32+IRecordInfo
ComInterface Interop+UiaCore+IRawElementProviderSimple
ComInterface Interop+Ole32+IStream
Class System.Windows.Forms.NativeMethods+PRINTDLGEX
Class System.Windows.Forms.NativeMethods+OPENFILENAME_I
Class System.Object
Class System.Byte[]
Class Microsoft.Win32.SafeHandles.CoTaskMemSafeHandle
Class System.Int32[]
UnmanagedStruct System.Guid
UnmanagedStruct Interop+Gdi32+HDC
UnmanagedStruct System.Drawing.Point
UnmanagedStruct System.Numerics.Matrix3x2
UnmanagedStruct Interop+RECT
UnmanagedStruct Interop+Gdi32+HRGN
UnmanagedStruct Interop+Gdi32+HGDIOBJ
UnmanagedStruct Interop+Gdi32+HPALETTE
UnmanagedStruct Interop+COLORREF
UnmanagedStruct Interop+Gdi32+HBITMAP
UnmanagedStruct System.Drawing.Size
UnmanagedStruct Interop+Gdi32+HENHMETAFILE
UnmanagedStruct Interop+Gdi32+HBRUSH
UnmanagedStruct Interop+Gdi32+HFONT
UnmanagedStruct Interop+Gdi32+HPEN
UnmanagedStruct Interop+Kernel32+LCID
UnmanagedStruct Interop+Atom
ManagedStruct Interop+Comdlg32+CHOOSECOLORW
ManagedStruct Interop+Comdlg32+CHOOSEFONTW
ManagedStruct Interop+Comdlg32+PAGESETUPDLGW
ManagedStruct Interop+Gdi32+BITMAPINFO
ManagedStruct System.Runtime.InteropServices.ComTypes.STGMEDIUM
ManagedStruct Interop+Oleaut32+FONTDESC
ManagedStruct Interop+Shell32+BROWSEINFO
ManagedStruct Interop+User32+DEVMODEW
ManagedDelegate Interop+Gdi32+Enhmfenumproc
ManagedDelegate Interop+User32+HOOKPROC
ManagedDelegate Interop+User32+MONITORENUMPROC

I would not claim that I copy all rules for LibraryImportGenerator, but seems to be I'm pretty close.

Not sure about which scenario runtime team think to implement, but I think it was what I call UnmanagedStruct (in the docs they are called Transparent Structures. In WinForms codebase there only 16 types like this, and even this is mundane, it seems manageable to implement. I overestimate complexity initially. Anyway, would be good do nothing in this cases.

Also not sure should DisableRuntimeMarshallingAttribute be applied for marshaling Transparent Structures.

Applying DisableRuntimeMarshallingAttribute should be last step, as it is require ComWrappers to be fully implemented across codebase. It is not clear from documentation, would it be possible to introduce changes piece meal or not.

It is not clear from documentation what is considered not strictly blittable. I do not get what's the difference between blittable, except all strictly blittable are should originate from same assembly where source generator runs. Please correct me.

Strictly blittable types seems to be a way how to postpone applying DisableRuntimeMarshallingAttribute at least in some parts. Is this correct?

/cc @gpetrou because you are driving changes in that direction now.

kant2002 avatar May 15 '22 14:05 kant2002

Applying DisableRuntimeMarshallingAttribute should be last step

Right.

I do not get what's the difference between blittable, except all strictly blittable

The idea is basically the old definition but limited to value types, classes marked sequential are not blittable, and must be made up of types that are either all defined in the same assembly or of always blittable primitives.

Strictly blittable types seems to be a way how to postpone applying DisableRuntimeMarshallingAttribute at least in some parts. Is this correct?

Sort of. There are a number of cases where DisableRuntimeMarshallingAttribute will cause a lot of complexity to change and although it might be worth it in the long run, it isn't needed for the majority of cases. One could think of it as a transition tool, but disabling runtime marshalling across the board because one needs to marshal something with two short fields shouldn't be the forcing function.

In WinForms codebase there only 16 types like this, and even this is mundane, it seems manageable to implement. I overestimate complexity initially. Anyway, would be good do nothing in this cases.

Right. This was our assumption. Generating marshallers for these types is expected to be rather simple and helps owners to be more active in the interop space rather than keeping it a mystery. If someone wants to perform some non-trivial interop they should be expected to participate at least a little.

AaronRobinsonMSFT avatar May 15 '22 16:05 AaronRobinsonMSFT

@merriemcgaw is this still valid with the move to using CsWin32?

elachlan avatar Jan 18 '23 00:01 elachlan

/cc @JeremyKuhne

AaronRobinsonMSFT avatar Jan 18 '23 00:01 AaronRobinsonMSFT

Yes, this is no longer relevant. Thanks for pointing it out, @elachlan.

JeremyKuhne avatar Jan 18 '23 07:01 JeremyKuhne