Skip to content

Remove StringBuilderCache from Version.ToString #48511

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.

Sign up for GitHub

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

Jump to bottom
Merged
stephentoub merged 1 commit into from
Feb 19, 2021
Merged

Remove StringBuilderCache from Version.ToString #48511

stephentoub merged 1 commit into from
Feb 19, 2021

Conversation

@stephentoub
Copy link
Member

@stephentoub stephentoub commented Feb 19, 2021

Rather than using a StringBuilder in both ToString and TryFormat, we can just write ToString in terms of TryFormat and use stack space for the small string being generated. As part of this, I removed the extra code optimizing ToString(0) and ToString(1), as I can't find any real code anywhere that does that and it's not worth the extra lines. Less code and faster.

Method Toolchain Mean Ratio
ToString2 \master\corerun.exe 44.20 ns 1.00
ToString2 \pr\corerun.exe 29.97 ns 0.68
ToString3 \master\corerun.exe 55.63 ns 1.00
ToString3 \pr\corerun.exe 36.81 ns 0.66
ToString4 \master\corerun.exe 64.09 ns 1.00
ToString4 \pr\corerun.exe 44.72 ns 0.70
ToStringL \master\corerun.exe 105.74 ns 1.00
ToStringL \pr\corerun.exe 86.57 ns 0.82
TryFormat2 \master\corerun.exe 44.05 ns 1.00
TryFormat2 \pr\corerun.exe 19.40 ns 0.44
TryFormat3 \master\corerun.exe 53.56 ns 1.00
TryFormat3 \pr\corerun.exe 26.88 ns 0.50
TryFormat4 \master\corerun.exe 64.35 ns 1.00
TryFormat4 \pr\corerun.exe 34.20 ns 0.53
TryFormatL \master\corerun.exe 101.31 ns 1.00
TryFormatL \pr\corerun.exe 70.57 ns 0.70 
private Version _v2 = new Version(1, 2);
private Version _v3 = new Version(1, 2, 3);
private Version _v4 = new Version(1, 2, 3, 4);
private Version _vL = new Version(int.MaxValue, int.MaxValue, int.MaxValue, int.MaxValue);
private char[] _buffer = new char[100];

[Benchmark] public string ToString2() => _v2.ToString();
[Benchmark] public string ToString3() => _v3.ToString();
[Benchmark] public string ToString4() => _v4.ToString();
[Benchmark] public string ToStringL() => _vL.ToString();

[Benchmark] public bool TryFormat2() => _v2.TryFormat(_buffer, out _);
[Benchmark] public bool TryFormat3() => _v3.TryFormat(_buffer, out _);
[Benchmark] public bool TryFormat4() => _v4.TryFormat(_buffer, out _);
[Benchmark] public bool TryFormatL() => _vL.TryFormat(_buffer, out _);

@stephentoub
Remove StringBuilderCache from Version.ToString
dc4f0b5 
Rather than using a StringBuilder in both ToString and TryFormat, we can just write ToString in terms of TryFormat and use stack space for the small string being generated.  As part of this, I removed the extra code optimizing ToString(0) and ToString(1), as I can't find any real code anywhere that does that and it's not worth the extra lines.
@ghost
Copy link

ghost commented Feb 19, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@stephentoub stephentoub added this to the 6.0.0 milestone Feb 19, 2021
jkotas
jkotas approved these changes Feb 19, 2021
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Nice!

@stephentoub stephentoub merged commit 1d2b40b into dotnet:master Feb 19, 2021
@stephentoub stephentoub deleted the versionsb branch February 19, 2021 18:38
@danmoseley
Copy link
Member

danmoseley commented Feb 19, 2021

Worth putting these benchmarks into the perf repo? They apparently have no shortage of machine time.

@stephentoub
Copy link
Member Author

stephentoub commented Feb 19, 2021

Sure: dotnet/performance#1694

@ghost ghost locked as resolved and limited conversation to collaborators Mar 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotas jkotas jkotas approved these changes

Assignees

No one assigned

Labels

area-System.Runtime tenet-performance Performance related issue

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@stephentoub @danmoseley @jkotas