-
Notifications
You must be signed in to change notification settings - Fork 63
[Java.Interop.Tools.Cecil] use MemoryMappedFile in DirectoryAssemblyResolver #1103
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
Conversation
…esolver Context: dotnet/linker#1130 Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1783420 In dotnet/linker#1130, their version of `DirectoryAssemblyResolver` was reworked to use `MemoryMappedFile` in the .NET 6 timeframe. This commit ports these changes here, as we have MSBuild tasks in xamarin/xamarin-android that use `DirectoryAssemblyResolver`. Primarily, the `<GenerateJavaStubs />` MSBuild task uses `DirectoryAssemblyResolver` to iterate over types in assembly to emit Java source code and generate `AndroidManifest.xml`. The results of these changes in a `dotnet new maui` project, an initial clean build: GenerateJavaStubs = 1.318 s, 1 calls. GenerateJavaStubs = 1.254 s, 1 calls. Saving ~64ms or about ~5% in this example. The current version of the linker's resolver in .NET 8+ has moved to the dotnet/runtime repo at: https://github.com/dotnet/runtime/blob/cd7d006030a7feace9076fa275fb5bffc1bf4a90/src/tools/illink/src/linker/Linker/AssemblyResolver.cs
|
|
||
| public ICollection<string> SearchDirectories {get; private set;} | ||
|
|
||
| readonly List<MemoryMappedViewStream> viewStreams = new List<MemoryMappedViewStream> (); |
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.
Is there any chance this will be called on different threads?
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.
If you used the same DirectoryAssemblyResolver on different threads, I think it would already run into problems with Cecil. Such as: #43
Most of our usage is one per MSBuild task:
|
Testing this here: dotnet/android#7991 |
|
It appears there is some subtle problem here: Maybe we can't do this in cases we are also going to write to the |
|
@jonpryor I think the latest on dotnet/android#7991 is green. I restarted one lane because 1 test failed. |
Formatting Fixes
Changes: dotnet/java-interop@3c2a066...a91ae7f * dotnet/java-interop@a91ae7f9: [generator] improve *Implementor constructors (dotnet/java-interop#1105) * dotnet/java-interop@7d42864d: [Java.Interop.Tools.Cecil] DirectoryAssemblyResolver+MemoryMappedFile (dotnet/java-interop#1103) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Context: dotnet/linker#1130
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1783420
In dotnet/linker#1130, their version of
DirectoryAssemblyResolverwas reworked to useMemoryMappedFilein the .NET 6 timeframe. This commit ports these changes here, as we have MSBuild tasks in xamarin/xamarin-android that useDirectoryAssemblyResolver.Primarily, the
<GenerateJavaStubs />MSBuild task usesDirectoryAssemblyResolverto iterate over types in assembly to emit Java source code and generateAndroidManifest.xml.The results of these changes in a
dotnet new mauiproject, an initial clean build:Saving ~64ms or about ~5% in this example.
The current version of the linker's resolver in .NET 8+ has moved to the dotnet/runtime repo at:
https://github.com/dotnet/runtime/blob/cd7d006030a7feace9076fa275fb5bffc1bf4a90/src/tools/illink/src/linker/Linker/AssemblyResolver.cs