KEMBAR78
[Java.Interop.Tools.Cecil] use MemoryMappedFile in DirectoryAssemblyResolver by jonathanpeppers · Pull Request #1103 · dotnet/java-interop · GitHub
Skip to content

Conversation

@jonathanpeppers
Copy link
Member

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

…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> ();
Copy link
Contributor

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?

Copy link
Member Author

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:

https://github.com/xamarin/xamarin-android/blob/e4785b937f3c585997df672709a26ddfc146b8d4/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs#L100-L107

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Apr 25, 2023
@jonathanpeppers
Copy link
Member Author

Testing this here: dotnet/android#7991

@jonathanpeppers
Copy link
Member Author

It appears there is some subtle problem here:

(_GenerateJavaStubs target) -> 
Xamarin.Android.Common.targets(1514,3): error XAGJS7009: System.InvalidOperationException: Operation is not valid due to the current state of the object.
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Mono.Cecil.Cil.DefaultSymbolWriterProvider.GetSymbolWriter(ModuleDefinition module, String fileName) in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/cecil/Mono.Cecil.Cil/Symbols.cs:line 1032
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Mono.Cecil.ModuleWriter.GetSymbolWriter(ModuleDefinition module, String fq_name, ISymbolWriterProvider symbol_writer_provider, WriterParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/cecil/Mono.Cecil/AssemblyWriter.cs:line 157
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Mono.Cecil.ModuleWriter.Write(ModuleDefinition module, Disposable`1 stream, WriterParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/cecil/Mono.Cecil/AssemblyWriter.cs:line 117
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Mono.Cecil.ModuleWriter.WriteModule(ModuleDefinition module, Disposable`1 stream, WriterParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/cecil/Mono.Cecil/AssemblyWriter.cs:line 78
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Mono.Cecil.ModuleDefinition.Write(String fileName, WriterParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/cecil/Mono.Cecil/ModuleDefinition.cs:line 1160
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Mono.Cecil.AssemblyDefinition.Write(String fileName, WriterParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/cecil/Mono.Cecil/AssemblyDefinition.cs:line 160
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Xamarin.Android.Tasks.MarshalMethodsAssemblyRewriter.Rewrite(DirectoryAssemblyResolver resolver, List`1 targetAssemblyPaths, Boolean brokenExceptionTransitions)
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Xamarin.Android.Tasks.GenerateJavaStubs.Run(DirectoryAssemblyResolver res, Boolean useMarshalMethods)
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Xamarin.Android.Tasks.GenerateJavaStubs.RunTask()
Xamarin.Android.Common.targets(1514,3): error XAGJS7009:    at Microsoft.Android.Build.Tasks.AndroidTask.Execute() in /Users/builder/azdo/_work/1/s/xamarin-android/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AndroidTask.cs:line 25

    0 Warning(s)
    1 Error(s)

Maybe we can't do this in cases we are also going to write to the AssemblyDefinition?

@jonathanpeppers jonathanpeppers marked this pull request as draft April 25, 2023 21:43
@jonathanpeppers jonathanpeppers marked this pull request as ready for review April 26, 2023 21:02
@jonathanpeppers
Copy link
Member Author

@jonpryor I think the latest on dotnet/android#7991 is green. I restarted one lane because 1 test failed.

@jonpryor jonpryor merged commit 7d42864 into main May 2, 2023
@jonpryor jonpryor deleted the DirectoryAssemblyResolver branch May 2, 2023 18:25
jonpryor pushed a commit to dotnet/android that referenced this pull request May 8, 2023
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>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants