Skip to content
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

Longer strings for string.Replace(char, char) benchmarks #2329

Merged
merged 3 commits into from
Mar 24, 2022
Merged

Longer strings for string.Replace(char, char) benchmarks #2329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0762c4
Longer strings for string.Replace(char, char) benchmarks
gfoidl Mar 23, 2022
5832037
Update src/benchmarks/micro/libraries/System.Runtime/Perf.String.cs
gfoidl Mar 23, 2022
4d629c9
Use a random string
gfoidl Mar 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/benchmarks/micro/libraries/CommonUtils/PerfUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.IO;
using System.Linq;

namespace System
{
Expand Down Expand Up @@ -31,5 +32,15 @@ public static string CreateString(int length)

return new string(str);
}

/// <summary>
/// Helper method to create a string containing random alphanumeric
/// characters equal to the specified length
/// </summary>
public static string CreateRandomString(int length, string alphabet = "abcdefghijklmnopqrstuvwxyz0123456789", int seed = 42)
{
Random random = new(seed); // use the given seed, to make the benchmarks repeatable
return new string(Enumerable.Repeat(alphabet, length).Select(s => s[random.Next(s.Length)]).ToArray());
}
}
}
15 changes: 11 additions & 4 deletions src/benchmarks/micro/libraries/System.Runtime/Perf.String.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,20 @@ public string TrimEnd_CharArr(string s, char[] c)
=> s.TrimEnd(c);

[Benchmark]
[Arguments("Hello", 'l', '!')] // Contains two 'l'
[Arguments("Hello", 'a', 'b')] // Contains one 'a'
[Arguments("This is a very nice sentence", 'z', 'y')] // 'z' does not exist in the string
[Arguments("This is a very nice sentence", 'i', 'I')] // 'i' occuress 3 times in the string
[ArgumentsSource(nameof(ReplaceArguments))]
public string Replace_Char(string text, char oldChar, char newChar)
=> text.Replace(oldChar, newChar);

public static IEnumerable<object[]> ReplaceArguments()
Copy link
Member Author

Choose a reason for hiding this comment

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

Verifyed that the method names are displayed the same way, so that the reporting tool has correlation. Is this enough to be on the safe side here?

|          Method |                 text | oldChar | newChar |       Mean |      Error |    StdDev |
|---------------- |--------------------- |-------- |-------- |-----------:|-----------:|----------:|
| Replace_CharNew |  ----(...)---- [100] |       - |       + |  32.553 ns |  21.554 ns | 1.1815 ns |
| Replace_CharNew | ----(...)---- [1000] |       - |       + | 202.048 ns | 178.291 ns | 9.7727 ns |
|    Replace_Char |                Hello |       a |       b |   5.256 ns |   4.023 ns | 0.2205 ns |
| Replace_CharNew |                Hello |       a |       b |   5.367 ns |   2.158 ns | 0.1183 ns |
|    Replace_Char |                Hello |       l |       ! |  17.491 ns |   4.643 ns | 0.2545 ns |
| Replace_CharNew |                Hello |       l |       ! |  18.092 ns |  12.480 ns | 0.6841 ns |
|    Replace_Char | This (...)tence [28] |       i |       I |  36.695 ns |  91.723 ns | 5.0277 ns |
| Replace_CharNew | This (...)tence [28] |       i |       I |  30.385 ns |  25.207 ns | 1.3817 ns |
|    Replace_Char | This (...)tence [28] |       z |       y |  11.598 ns |   6.366 ns | 0.3489 ns |
| Replace_CharNew | This (...)tence [28] |       z |       y |  12.132 ns |  10.304 ns | 0.5648 ns |

The ones with suffix New are the methods with the ArgumentsSource, the other ones with Arguments only (as in main-branch).

Copy link
Member

Choose a reason for hiding this comment

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

@adamsitnik @DrewScoggins can you confirm this won't reset the results?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I thought I had already responded here. As long as the test name or the arguments have not changed values we should have no issues here. And from what I can tell nothing has changed for the existing test cases. So this LGTM.

{
yield return new object[] { "Hello", 'l', '!' }; // Contains two 'l'
yield return new object[] { "Hello", 'a', 'b' }; // Contains one 'a'
yield return new object[] { "This is a very nice sentence", 'z', 'y' }; // 'z' does not exist in the string
yield return new object[] { "This is a very nice sentence", 'i', 'I' }; // 'i' occurs 3 times in the string
yield return new object[] { PerfUtils.CreateRandomString(100, seed: 42), 'b', '+' }; // b occurs 8 times in the string
yield return new object[] { PerfUtils.CreateRandomString(1000, seed: 42), 'b', '+' }; // b occurs 42 times in the string
}

[Benchmark]
[Arguments("This is a very nice sentence", "bad", "nice")] // there are no "bad" words in the string
[Arguments("This is a very nice sentence", "nice", "bad")] // there are is one "nice" word in the string
Expand Down