Repository navigation
RFC: Use AdvancedDLSupport instead of P/Invoke or GetFunctionPointerForDelegate #987
Description
While investigating how to proceed on binding libpython I stumbled over this post: https://medium.com/@jarl.gullberg/an-introduction-to-adl-or-how-to-double-your-native-net-interop-performance-c008e4da54db
It showcases https://github.com/Firwood-Software/AdvancedDLSupport and also benchmarks the individual options, which look quite devastating for the combination GetFunctionPointerForDelegate+Mono and at least "concerning" for the other frameworks.
The library looks really slick and solves exactly our problem while (seemingly) delivering slightly better performance. It has however the catch of being LGPL. While this is perfectly fine for me, this means there might be a problem for applications trying to embed Python.Runtime ahead-of-time compiled or publishing to a closed app-store that prevents you from exchanging the LGPLed library locally.
Comments?
Activity
lostmsu commentedon Nov 13, 2019
I am a bit concerned about
I don't know how much is this applicable to our users. We should discuss at the next call.
Additionally, reading this I wonder if the calling convention is the reason for crash in #980 @Sngmng
filmor commentedon Nov 14, 2019
This is a pure .NET Core problem AFAIK (and the article is from 2018, it could be that this is fixed on .NET Core 3). Also ADL allows you to choose the style and fall back to delegates for unsupported cases.
thesn10 commentedon Nov 14, 2019
Well they mentioned if you dont want LGPL you can still ask them for a custom license:
"For companies, established open-source projects, or individuals wanting a custom license, please chuck us an email."
This could be the issue, maybe. Should i try to use LoadLibrary and GetProcAdress instead?
lostmsu commentedon Nov 14, 2019
@Sngmng you can try fiddling with DllImportAttribute.CallingConvention
lostmsu commentedon Nov 22, 2019
I started prototyping Python C API interop layer: https://github.com/losttech/Python.Runtime.Native/tree/InterfaceBasedApiSets
Run my own tests on Windows for
DllImportvsGetDelegateForFunctionPointerand results came a bit surprising:GetDelegateForFunctionPointeris 2x faster thanDllImporton CoreRT and 2x slower on regular .NET Framework (for a function with no return value and a pointer parameter).lostmsu commentedon Nov 22, 2019
Hm, nevermind, longer reruns normalize it to 1.1-2.0x on all platforms.
For some reason BenchmarkDotNet did not pick up my local Mono installation.
lostmsu commentedon Nov 22, 2019
After some tweaking this is what I got (Calli through AdvancedDLSupport):
filmor commentedon Nov 22, 2019
Thank you very much for running these :)
What is the difference between CoreRt 3.0 and .NET Core 3.0 here?
/edit: nvm, it's clear now :)
0xBEEBAD commentedon Nov 23, 2019
This might solve the problem of Unity AOT compatibility issue
lostmsu commentedon Nov 24, 2019
@sekkit which issue are you referring to?
BTW, I prototyped and benchmarked a simple PInvoke generator called
EmitBinder, that usesReflection.Emit(for AOT,Marshal.GetDelegateForFunctionPointerseem to be a necessary fallback). Added it to the benchmark:lostmsu commentedon Nov 24, 2019
Finally, tested on Linux x64 (this one is in VM, so results might be inaccurate):
lostmsu commentedon Nov 24, 2019
Actually, I think the way to go forward is pretty straightforward:
runtime.csMarshal.GetDelegateForFunctionPointerimplementation, because it works on all platforms (the only one, that works with CoreRT), has no extra dependencies in .NET Standard 2.0, and is only really slow on .NET Core 3.0 (what a surprise!)EmitBinderand/orCalli. The later does not have to useAdvancedDLSupportpackage, as implementation would be very similar toEmitBinder, so probably under 100 lines of code.filmor commentedon Nov 24, 2019
Sounds like a good plan, but I think if it's not too much work, we should have all of this directly in the main repository (at least after a bit of experimentation).
I suggest we start a
modernisationbranch that is supposed to be 2.5 in the end (or 3.0 if we break backwards-compatibility too much) where we do only the following:clrmoduleandmonoclrconsolePython.Runtime,Python.Runtime.Interop, embed and perf tests, helper library for python tests, all of these under a commonPython.Testnamespace)pythonnetdirectoryThat way we can do major changes to the architecture without dropping all of the current PRs. As long as we don't change
Python.Runtimeitself significantly, most of them can probably be reapplied manually for 2.5.filmor commentedon Nov 27, 2019
I started this now, are you okay with me merging your work into the branch?
0xBEEBAD commentedon Nov 28, 2019
#997
@filmor Would u consider removing Reflection.Emit in Modernisation branch?
lostmsu commentedon Nov 28, 2019
@sekkit would Unity AOT allow choosing Emit vs non-Emit implementation at runtime? The former is required for better performance.
0xBEEBAD commentedon Nov 28, 2019
Reflectoin.Emit is not allowed. The above fork I posted: https://github.com/joonhwan/pythonnet is a AOT compatible implementation. No reflection overhead( fast-call branch)
lostmsu commentedon Nov 28, 2019
@sekkit it still relies on
DllImport, which we want to stop depending on.filmor commentedon Nov 28, 2019
Well, we can still depend on it for Mono and .NET Core as both implement the
__InternalDllImporttarget. It's just .NET Framework that requires a different approach.lostmsu commentedon Nov 28, 2019
Sure, go ahead.
But which work are you referring to?
lostmsu commentedon Nov 28, 2019
Wait, on Windows too?
filmor commentedon Nov 28, 2019
Ah, sorry, I again fell for CoreRt vs CoreClr :/
Nevertheless, Mono and CoreRt support
__Internaland CoreClr has at least an open issue on this (https://github.com/dotnet/coreclr/issues/9038) and https://github.com/dotnet/corefx/issues/32015 which (as far as I understand) allows us to just hook into the library loading mechanism to make__Internalpoint at the Python DLL of our liking.filmor commentedon Feb 4, 2021
Since the unmanaged delegate changes are now merged, this one is obsolete.