-
Notifications
You must be signed in to change notification settings - Fork 50
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
[JavaCallableWrappers] avoid string.Format()
#1061
Conversation
Running PerfView on Windows in a .NET MAUI app, I can see this taking up time inside the `<GenerateJavaStubs/>` MSBuild task: | Name | Inc % | Inc | | --- | --: | --: | | java.interop.tools.javacallablewrappers!Java.Interop.Tools.JavaCallableWrappers.JavaCallableWrapperGenerator..ctor() | 1.4 | 51 | | mscorlib.ni!String.Format | 1.0 | 35 | Around ~26ms appear to be spent inside `JavaCallableWrapperGenerator` doing `string.Format()`. Reviewing the code, there is quite a bit of `WriteLine()` used along with format arguments. I could rework this to use string interpolation, which would be slightly better -- as the C# compiler could replace many of these with `string.Concat()`. However, the best case is to just call `Write()` with individual strings, so we don't do extra work creating intermediate string values. Copying `Java.Interop.Tools.JavaCallableWrappers.dll` to: C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\33.0.4\tools It appears to save about ~17ms in a `dotnet new maui` project: Top 10 most expensive tasks --GenerateJavaStubs = 795 ms ++GenerateJavaStubs = 812 ms This was an incremental build with a `.xaml` change.
@@ -574,8 +576,7 @@ public void Generate (TextWriter writer) | |||
|
|||
if (children != null) { | |||
for (int i = 0; i < children.Count; ++i) { | |||
string methods = string.Format ("__md_{0}_methods", i + 1); | |||
GenerateRegisterType (writer, children [i], methods); | |||
GenerateRegisterType (writer, children [i], $"__md_{i + 1}_methods"); |
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.
While I can see multiple .Write()
calls being faster -- and that makes up the majority of this PR -- are format strings actually faster than string.Format()
?
For a quick one-off, it looks like the IL for these two statements is identical:
int i = 1;
var s1 = string.Format ("__md_{0}_methods", i);
var s2 = $"__md_{i}_methods";
Both s1
and s2
become string.Format()
calls. Thus, this doesn't appear to be a significant change.
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.
Were you testing .NET 7?
The second one does this for me:
DefaultInterpolatedStringHandler defaultInterpolatedStringHandler = new DefaultInterpolatedStringHandler(13, 1);
defaultInterpolatedStringHandler.AppendLiteral("__md_");
defaultInterpolatedStringHandler.AppendFormatted(i);
defaultInterpolatedStringHandler.AppendLiteral("_methods");
Console.WriteLine(defaultInterpolatedStringHandler.ToStringAndClear());
It seems like you might also need to use s1
/s2
.
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.
Changes: xamarin/java.interop@3a9f770...149d70f * xamarin/java.interop@149d70fe: [generator] Refactor enum writing to use SourceWriters (xamarin/java.interop#1063) * xamarin/java.interop@c2daa9f0: [Java.Interop.Tools.Cecil] DirectoryAssemblyResolver & File.Exists() (xamarin/java.interop#1065) * xamarin/java.interop@8ab9d33a: [Java.Interop.Tools.TypeNameMappings] improve `ToJniNameFromAttributesForAndroid` (xamarin/java.interop#1064) * xamarin/java.interop@09f8da26: [JavaCallableWrappers] avoid `string.Format()` (xamarin/java.interop#1061) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Running PerfView on Windows in a .NET MAUI app, I can see this taking up time inside the `<GenerateJavaStubs/>` MSBuild task: | Name | Inc % | Inc | | --------------------------------------------------------------------------------------------------------------------- | ----: | ----: | | java.interop.tools.javacallablewrappers!Java.Interop.Tools.JavaCallableWrappers.JavaCallableWrapperGenerator..ctor() | 1.4 | 51 | | mscorlib.ni!String.Format | 1.0 | 35 | Around ~26ms appears to be spent inside `JavaCallableWrapperGenerator` calling `string.Format()`. Reviewing the code, there is quite a bit of `WriteLine()` used along with format arguments. I could rework this to use string interpolation, which would be slightly better, as the C# compiler could replace many of these with `string.Concat()`. However, the best case is to just call `Write()` with individual strings, so we don't do extra work creating intermediate string values. Copying `Java.Interop.Tools.JavaCallableWrappers.dll` to: C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\33.0.4\tools It appears to save about ~17ms in a `dotnet new maui` project: Top 10 most expensive tasks --GenerateJavaStubs = 795 ms ++GenerateJavaStubs = 812 ms This was an incremental build with a `.xaml` change.
Running PerfView on Windows in a .NET MAUI app, I can see this taking up time inside the
<GenerateJavaStubs/>
MSBuild task:Around ~35ms appear to be spent inside
JavaCallableWrapperGenerator
doingstring.Format()
.Reviewing the code, there is quite a bit of
WriteLine()
used along with format arguments. I could rework this to use string interpolation, which would be slightly better -- as the C# compiler could replace many of these withstring.Concat()
.However, the best case is to just call
Write()
with individual strings, so we don't do extra work creating intermediate string values.Copying
Java.Interop.Tools.JavaCallableWrappers.dll
to:It appears to save about ~17ms in a
dotnet new maui
project:This was an incremental build with a
.xaml
change.