-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Merge feature/use-dllimport-generator to main #59579
Conversation
- Includes identification of GeneratedDllImportAttribute - Generation of enclosing types for partial methods - Basic tests for compilation of generation code Commit migrated from dotnet/runtimelab@56076d3
calls directly to the real DllImport from the generated one. Add test for handling RefKind (i.e. in, ref, out). Commit migrated from dotnet/runtimelab@3670643
- User defined source needs to be "correct" Commit migrated from dotnet/runtimelab@f9f705c
It is interesting to step into the generated method and observe the UX. Commit migrated from dotnet/runtimelab@35c9adc
* Update readme with existing experiments * Add "CreateAnExperiment.md" Commit migrated from dotnet/runtimelab@542dd85
Commit migrated from dotnet/runtimelab@4072ab9
the attribute name. Add test cases for compilation failures. Commit migrated from dotnet/runtimelab@d2f6f5a
…e_test_case Failure test cases. Commit migrated from dotnet/runtimelab@48b8361
…melab#36) * Collect DllImport details through Roslyn semantic model Create an assembly that provides the GeneratedDllImportAttribute which allows use of the semantic model. * Collect MarshalAsAttribute data * Collect MarshalAsAttribute data Convert P/Invoke centric data structures to handle positional type information for managed/unmanaged scenarios. Commit migrated from dotnet/runtimelab@21e634e
Co-authored-by: Jan Kotas <[email protected]> Commit migrated from dotnet/runtimelab@e50c8c2
… analyzer that validates manual usage. (dotnet/runtimelab#61) Co-authored-by: Elinor Fung <[email protected]> Commit migrated from dotnet/runtimelab@05d6ef2
… to .NET 5 support) (dotnet/runtimelab#80) Commit migrated from dotnet/runtimelab@91b131e
Commit migrated from dotnet/runtimelab@4eaa5d8
…g types. (dotnet/runtimelab#108) Commit migrated from dotnet/runtimelab@4bb5cb0
…oc constructor exists. (dotnet/runtimelab#125) Commit migrated from dotnet/runtimelab@9e7e221
* Use DNNE to create native testing assets Instead of relying on a native project use DNNE to generate native exports for managed binaries. This allows us to avoid writing native code for testing code generation. * Add *.binlog to .gitignore * Force 'UseAppHost' to fix scenario on macOS. * Consume support in DNNE for adding native binary to project. Commit migrated from dotnet/runtimelab@415c6ca
Commit migrated from dotnet/runtimelab@8497669
Co-authored-by: Jan Kotas <[email protected]> Commit migrated from dotnet/runtimelab@2cde5aa
* Rename DllImportGenerator.Test to DllImportGenerator.UnitTests * Create an integration tests project. Update the SLN file. Remove SafeHandle test from Demo project. * Update readme.md Make the Demo project a user friendly example for learning about the methodology. Commit migrated from dotnet/runtimelab@778ecfa
* Boolean and Delegate tests Updated to DNNE 1.0.12 with support for C# function pointers. Renamed CBoolMarshaller to ByteBoolMarshaller. Added integration tests for all bool marshaller types. Added integration tests for delegates. Style nits. Commit migrated from dotnet/runtimelab@6c980df
Commit migrated from dotnet/runtimelab@98f55be
* Support for char marshaller. Avoid supporting the built-in semantics for ANSI marshalling of char. Propagate the CharSet down to the marshallers. Convert the MarshallingGenerators.TryCreate() to a MarshallingGenerators.Create() function throwing an exception. * Add a compatibility document. * Add testing for char marshalling. Commit migrated from dotnet/runtimelab@be3f4e8
* Fix some warnings about analyzer authoring Commit migrated from dotnet/runtimelab@c89fc41
Commit migrated from dotnet/runtimelab@891d530
I've run the ASP.NET JSON macrobenchmark on this branch and compared it to main, and we see a minor performance improvement in steady state performance with this branch.
|
/cc @davidfowl – #59579 (comment). |
Ran some of the microbenchmarks that hit updated p/invokes to check that we aren't regressing them. On Windows with Perf_Console
Perf_Directory
Perf_File
Perf_FileStream
Perf_Hashing
Perf_NamedPipeStream
SslStreamTests
The file system ones I ran a bunch of times - from the historical results, it seems like there is quite a bit of variation. I had a mix of test cases flagged faster / slower, but not the same test case consistently. |
Is there a document stating short intro, rationale and/or benefit of this feature? The disadvantages appear to be: it increases code size in this repository, increases compiled assembly size and there is no significant gain in performance either. |
@kasperk81 here's the overall design doc for the feature: https://github.com/dotnet/runtime/blob/main/docs/design/features/source-generator-pinvokes.md The primary benefits of this feature are: better diagnostics (the marshalling stubs are actually debuggable), better AOT and linker compatibility (the AOT compiler doesn't need to hard-code implicit dependencies that the runtime uses to implement the built-in marshalling), and easier evolution (moving the marshalling infrastructure to run at compile time makes it easier for us to fix bugs since changes don't affect every P/Invoke that the user's application uses, even ones they didn't write). |
@kasperk81 This is a short term and intentional non-goal. The reasoning is we want to ensure a high level of confidence and compatibility for v1. To that end we did do some optimizations, but very few. As we gain confidence with how this methodology works we are free to try optimizations that are targeted as opposed to the "everyone gets it" design with the built-in system. As we get further into .NET 7 and onto .NET 8 we are considering opportunities for better performance in this area. |
@VSadov - looks like things are failing across the board (not just in this PR) - not sure why the CI in #59590 didn't fail 😕
|
/azp run runtime |
Azure Pipelines successfully started running 1 pipeline(s). |
…60374) * Update StructMarshalling.md with details Co-authored-by: Elinor Fung <[email protected]> Co-authored-by: Jeremy Koritzinsky <[email protected]>
* Remove dotnet-experimental feed from NuGet.config * Revert changes to solution files and restore configs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Merging with a merge commit as it was requested to preserve the history of commits coming from the experiment. |
Merge the DllImportGenerator experiment to the main branch for .NET 7.
This should be merged with "Create a merge commit"
ASP.NET JSON macro-benchmark: #59579 (comment)
Libraries micro-benchmarks: #59579 (comment)
Items we will address once the initial merge happens: #60212
cc @stephentoub