Skip to content

Add Perf_String_Substring_* benchmarks #2178

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

Closed
wants to merge 8 commits into from

Conversation

nietras
Copy link
Contributor

@nietras nietras commented Dec 10, 2021

Draft benchmarks for Substring. cc @adamsitnik feel free to make any changes you like right away. For some reason I can no longer built this after updating latest main. In relation to dotnet/runtime#62577

@@ -76,6 +76,7 @@ public string Remove_IntInt(string s, int i1, int i2)
[Arguments("dzsdzsDDZSDZSDZSddsz", 0)]
[Arguments("dzsdzsDDZSDZSDZSddsz", 7)]
[Arguments("dzsdzsDDZSDZSDZSddsz", 10)]
[Arguments("dzsdzsDDZSDZSDZSddsz", 19)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably will delete this change, so ignore it

// public void Substring_Full() => "abc".Substring(0, 3);

// [Benchmark]
// public void Substring_Empty() => "abc".Substring(0, 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure these are of any interest but they showed that with the new changes the JIT can fully compile away substring code compared to before

@nietras
Copy link
Contributor Author

nietras commented Dec 10, 2021

build errors

C:\git\oss\performance\src\benchmarks\micro\Serializers\Binary_FromStream.cs(39,13): error SYSLIB0011: 'BinaryFormatter.Serialize(Stream, object)' is obsolete: 'BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.' [C:\git\oss\performance\src\benchmarks\micro\MicroBenchmarks.csproj]
C:\git\oss\performance\src\benchmarks\micro\Serializers\Binary_FromStream.cs(61,23): error SYSLIB0011: 'BinaryFormatter.Deserialize(Stream)' is obsolete: 'BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.' [C:\git\oss\performance\src\benchmarks\micro\MicroBenchmarks.csproj]
C:\git\oss\performance\src\benchmarks\micro\Serializers\Binary_ToStream.cs(40,13): error SYSLIB0011: 'BinaryFormatter.Serialize(Stream, object)' is obsolete: 'BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.' [C:\git\oss\performance\src\benchmarks\micro\MicroBenchmarks.csproj]
C:\git\oss\performance\src\benchmarks\micro\libraries\System.Runtime.Serialization.Formatters\Perf.BinaryFormatter.cs(30,13): error SYSLIB0011: 'BinaryFormatter.Serialize(Stream, object)' is obsolete: 'BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.' [C:\git\oss\performance\src\benchmarks\micro\MicroBenchmarks.csproj]
C:\git\oss\performance\src\benchmarks\micro\libraries\System.Runtime.Serialization.Formatters\Perf.BinaryFormatter.cs(38,32): error SYSLIB0011: 'BinaryFormatter.Deserialize(Stream)' is obsolete: 'BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.' [C:\git\oss\performance\src\benchmarks\micro\MicroBenchmarks.csproj]

@danmoseley
Copy link
Member

Not sure why those build failures don't show up when I build locally. There should be something like this suppressing them though.
https://github.com/dotnet/runtime/blob/03e90a540cb2dfe5cab4086ea54ad5dd1f655749/src/libraries/System.Runtime/tests/System/ExceptionTests.cs#L238-L237

@nietras
Copy link
Contributor Author

nietras commented Dec 11, 2021

Suppressed the warning in the places this was missing. See PR. Then getting assets issue:

C:\Program Files\dotnet\sdk\7.0.100-alpha.1.21568.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(267,5): error NETSDK1005: Assets file 'C:\git\oss\performance\artifacts\obj\MicroBenchmarks\project.assets.json' doesn't have a target for 'net6.0'. Ensure that restore has run and that you have included 'net6.0' in the TargetFrameworks for your project. [C:\git\oss\performance\src\benchmarks\micro\MicroBenchmarks.csproj]

so switched to netcoreapp5.0 in compilation. Tried rebuilding, cleaning, restoring didn't help. Didn't have these issues before updating to latest main.

@nietras
Copy link
Contributor Author

nietras commented Dec 14, 2021

Reported some results in dotnet/runtime#62577 (comment)

@danmoseley
Copy link
Member

@nietras checking in on this one, do you believe it's ready to review (after resolving conflicts)?

@danmoseley danmoseley force-pushed the perf-string-substring branch from a668e00 to 6e1617c Compare February 8, 2022 17:16
@danmoseley
Copy link
Member

@nietras I'm closing this one as it seems that string.Substring(int) and string.Substring(int, int) are already covered very similarly. Please let me know if that's not the case.

@danmoseley danmoseley closed this Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants