Merge feature/use-dllimport-generator to main #59579
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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 <jkotas@microsoft.com> Commit migrated from dotnet/runtimelab@e50c8c2
… analyzer that validates manual usage. (dotnet/runtimelab#61) Co-authored-by: Elinor Fung <elfung@microsoft.com> 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 <jkotas@microsoft.com> 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). |
13 tasks
|
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 <elfung@microsoft.com> Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
* Remove dotnet-experimental feed from NuGet.config * Revert changes to solution files and restore configs
elinor-fung
approved these changes
Oct 16, 2021
AaronRobinsonMSFT
approved these changes
Oct 18, 2021
|
Merging with a merge commit as it was requested to preserve the history of commits coming from the experiment. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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