Skip to content
This repository was archived by the owner on Aug 27, 2026. It is now read-only.

[JavaCallableWrappers] avoid string.Format() - #1061

Merged
jonpryor merged 1 commit into
mainfrom
jcwgen-string.format
Dec 6, 2022
Merged

jonpryor merged 1 commit into
mainfrom
jcwgen-string.format

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

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 ~35ms 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.

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.
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");

Copy link
Copy Markdown
Contributor

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.

@jonathanpeppers jonathanpeppers Dec 5, 2022 •

Copy link
Copy Markdown
Member Author

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathanpeppers wrote:

Were you testing .NET 7?

Oops. Nope. :-(

@jonpryor
jonpryor merged commit 09f8da2 into main Dec 6, 2022
@jonpryor
jonpryor deleted the jcwgen-string.format branch December 6, 2022 21:12
jonpryor pushed a commit to dotnet/android that referenced this pull request Dec 12, 2022
Changes: dotnet/java-interop@3a9f770...149d70f

  * dotnet/java-interop@149d70fe: [generator] Refactor enum writing to use SourceWriters (dotnet/java-interop#1063)
  * dotnet/java-interop@c2daa9f0: [Java.Interop.Tools.Cecil] DirectoryAssemblyResolver & File.Exists() (dotnet/java-interop#1065)
  * dotnet/java-interop@8ab9d33a: [Java.Interop.Tools.TypeNameMappings] improve `ToJniNameFromAttributesForAndroid` (dotnet/java-interop#1064)
  * dotnet/java-interop@09f8da26: [JavaCallableWrappers] avoid `string.Format()` (dotnet/java-interop#1061)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
jonathanpeppers added a commit that referenced this pull request Jan 5, 2023
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.
@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.

2 participants