[Xamarin.Android.Build.Tasks] perf improvements for LlvmIrGenerator #7626
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
Reviewing `dotnet trace` output for a `dotnet new maui` app:
dotnet trace collect --format speedscope -- .\bin\Release\dotnet\dotnet.exe build -bl --no-restore foo.csproj
21% of the time in `<GenerateJavaStubs/>` is spent in:
249.60ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.Andro...
Drilling down, some of the hot spots are:
68.64ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WriteString(class System.String,class System.St
40.39ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WritePointer(class Xamarin.Android.Tasks.LLVMIR
Which seem to be places
The types of things I solved so far:
* I audited every `$""` to see if we could call `TextWriter.Write()`
or `StringBuilder.Append()` multiple times to avoid creating a
string.
* Use `ArrayPool` in combination with `Encoding.GetBytes()`.
* Use `char` overloads, where we were using a 1-length `string`.
Hopefully, I've changed no behavior at all here, and we've just saved
a lot of `string` and `byte[]` allocations.
After these changes go in, we can probably profile and find other
areas to improve.
Testing the same `dotnet new maui` app, the overall time is now:
190.97ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.AndroidT
I think this will save ~60ms on incremental builds of the .NET MAUI
project template. Larger projects with more Java subclasses could save
even more.
grendello
approved these changes
Dec 9, 2022
dellis1972
approved these changes
Dec 9, 2022
I like |
The main reason I used it was to be able to see what syntax is generated by the line. I wonder, maybe a comment with target syntax above the relevant code would be a good idea? |
jonathanpeppers
commented
Dec 9, 2022
Comment on lines
479
to
485
| output.WriteLine ($"[{bufferSize} x {irType}] zeroinitializer, align {GetAggregateAlignment ((int)smi.BaseTypeSize, size)}"); | ||
| output.Write ('['); | ||
| output.Write (bufferSize); | ||
| output.Write (" x "); | ||
| output.Write (irType); | ||
| output.Write ("] zeroinitializer, align "); | ||
| output.WriteLine (GetAggregateAlignment ((int) smi.BaseTypeSize, size)); |
There was a problem hiding this comment.
For this line, for example, I could add a comment like:
// $"[{bufferSize} x {irType}] zeroinitializer, align {GetAggregateAlignment ((int)smi.BaseTypeSize, size)}"I'll go through and do that, but let's see if the PR has any failing tests or anything.
Adding some comments where `$""` was more readable before
Classic tests were failing with `unable to load assembly System.Buffers.dll` on Windows, because they used the `GeneratePackageManagerJava` MSBuild task directly from C# in a .NET framework test project. To fix this, I brought over an assembly-binding redirect we already had in xabuild's `App.config`.
grendello
added a commit
to grendello/xamarin-android
that referenced
this pull request
Jan 4, 2023
* main: [Xamarin.Android.Build.Tasks] use %(TrimmerRootAssembly.RootMode) All (dotnet#7651) Bump to dotnet/installer@47a747f 8.0.100-alpha.1.22616.7 (dotnet#7647) Bump to dotnet/installer@167a4ed 8.0.100-alpha.1.22611.1 (dotnet#7630) Bump to xamarin/Java.Interop/main@f8d77fa (dotnet#7638) [ci] Fix Designer test paths (dotnet#7635) [Xamarin.Android.Build.Tasks] perf improvements for LlvmIrGenerator (dotnet#7626) [Xamarin.Android.Build.Tasks] AutoImport `*.webp` files (dotnet#7601) Localized file check-in by OneLocBuild Task (dotnet#7632) Bump $(ProductVersion) to 13.2.99 Bump to xamarin/monodroid@c0c933b7 (dotnet#7633) [Xamarin.Android.Build.Tasks] Fix aapt_rules.txt corruption (dotnet#7587) [Xamarin.Android.Build.Tasks] Add XA1031 error (dotnet#7448) Bump to xamarin/Java.Interop/main@149d70f (dotnet#7625) [CODEOWNERS] More updates to CODEOWNERS (dotnet#7628) [Xamarin.Android.Build.Tasks] avoid `File.Exists()` check (dotnet#7621)
jonathanpeppers
added a commit
that referenced
this pull request
Jan 5, 2023
…7626) Reviewing `dotnet trace` output for a `dotnet new maui` app: dotnet trace collect --format speedscope -- .\bin\Release\dotnet\dotnet.exe build -bl --no-restore foo.csproj 21% of the time in `<GenerateJavaStubs/>` is spent in: 249.60ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.Andro... Drilling down, some of the hot spots are: 68.64ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WriteString(class System.String,class System.St 40.39ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WritePointer(class Xamarin.Android.Tasks.LLVMIR Which seem to be places The types of things I solved so far: * I audited every `$""` to see if we could call `TextWriter.Write()` or `StringBuilder.Append()` multiple times to avoid creating a string. * Use `ArrayPool` in combination with `Encoding.GetBytes()`. * Use `char` overloads, where we were using a 1-length `string`. Hopefully, I've changed no behavior at all here, and we've just saved a lot of `string` and `byte[]` allocations. After these changes go in, we can probably profile and find other areas to improve. Testing the same `dotnet new maui` app, the overall time is now: 190.97ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.AndroidT I think this will save ~60ms on incremental builds of the .NET MAUI project template. Larger projects with more Java subclasses could save even more. Other changes: * Added comments where `$""` was more readable before * Fix new usage of System.Buffers in tests Classic tests were failing with `unable to load assembly System.Buffers.dll` on Windows, because they used the `GeneratePackageManagerJava` MSBuild task directly from C# in a .NET framework test project. To fix this, I brought over an assembly-binding redirect we already had in xabuild's `App.config`.
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.
Reviewing
dotnet traceoutput for adotnet new mauiapp:21% of the time in
<GenerateJavaStubs/>is spent in:Drilling down, some of the hot spots are:
These seem to be places we create intermediate
stringorbyte[]objects.The types of things I solved so far:
I audited every
$""to see if we could callTextWriter.Write()orStringBuilder.Append()multiple times to avoid creating a string.Use
ArrayPoolin combination withEncoding.GetBytes().Use
charoverloads, where we were using a 1-lengthstring.Hopefully, I've changed no behavior at all here, and we've just saved a lot of
stringandbyte[]allocations.After these changes go in, we can probably profile and find other areas to improve.
Testing the same
dotnet new mauiapp, the overall time is now:I think this will save ~60ms on incremental builds of the .NET MAUI project template. Larger projects with more Java subclasses could save even more.