Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Remove a small allocation in MethodBase.ToString() - #25631

Merged
jkotas merged 1 commit into
dotnet:masterfrom
EgorBo:MethodBase-to-string-alloc
Aug 28, 2019
Merged

jkotas merged 1 commit into
dotnet:masterfrom
EgorBo:MethodBase-to-string-alloc

Conversation

@EgorBo

@EgorBo EgorBo commented Jul 10, 2019

Copy link
Copy Markdown
Member

Don't call base.ToString() and Substring if the type is void (the most popular return type I guess).

MethodInfo method = typeof(Program).GetMethod("Main");
long a = GC.GetAllocatedBytesForCurrentThread();

string str = method.ToString(); // "Void Main(System.String[])"

long b = GC.GetAllocatedBytesForCurrentThread();
Console.WriteLine((b - a) + " bytes");

Before: 3792 bytes
After: 3592 bytes

I know the ToString() for methods is cached so this optimization affects only first calls of methodBase.ToString().

@MichalStrehovsky

Copy link
Copy Markdown
Member

Why not just make the primitives, void, etc. take the IsNested path that just returns the Name? We could get rid of the Substring completely.

@EgorBo

EgorBo commented Jul 10, 2019

Copy link
Copy Markdown
Member Author

@MichalStrehovsky oh, indeed, however maybe it make sense to keep that fast path for "Void"?

@EgorBo

EgorBo commented Jul 10, 2019

Copy link
Copy Markdown
Member Author

Moved to the shared partitions in order to re-use it from Mono
AppendParameters is untouched.

@stephentoub

Copy link
Copy Markdown
Member

however maybe it make sense to keep that fast path for "Void"?

Why?

Comment thread src/System.Private.CoreLib/shared/System/RuntimeType.cs Outdated
@EgorBo

EgorBo commented Jul 11, 2019

Copy link
Copy Markdown
Member Author

Removed fast path for void.

Comment thread src/System.Private.CoreLib/shared/System/Reflection/MethodBase.cs
@jkotas

jkotas commented Aug 27, 2019

Copy link
Copy Markdown
Member

@EgorBo Could you please resolve the conflict?

@EgorBo

EgorBo commented Aug 28, 2019

Copy link
Copy Markdown
Member Author

@jkotas oh, sure!

@EgorBo
EgorBo force-pushed the MethodBase-to-string-alloc branch from 088c2d9 to 8c4e27a Compare August 28, 2019 14:31
@jkotas
jkotas merged commit f07c13c into dotnet:master Aug 28, 2019
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corefx that referenced this pull request Aug 28, 2019
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Aug 28, 2019
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request Aug 28, 2019
stephentoub pushed a commit to dotnet/corefx that referenced this pull request Aug 28, 2019
jkotas pushed a commit to dotnet/corert that referenced this pull request Aug 29, 2019
vargaz pushed a commit to mono/mono that referenced this pull request Sep 2, 2019
* Move FormatTypeName to Type (dotnet/coreclr#25631)

Signed-off-by: dotnet-bot <[email protected]>

* Fix for mono
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
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.

6 participants