-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Adding some basic perf tests for Double.TryParse and Single.TryParse #32392
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ public class Perf_Double | |
// NOTE: Consider duplicating any tests added here in Perf.Single.cs | ||
|
||
private volatile string _string; | ||
private volatile bool _bool; | ||
|
||
[Benchmark] | ||
[InlineData(double.NegativeInfinity, 10_000_000)] // Negative Infinity | ||
|
@@ -49,6 +50,40 @@ public void DefaultToString(double number, int innerIterations) | |
} | ||
} | ||
|
||
[Benchmark] | ||
[InlineData("-∞", 10_000_000)] // Negative Infinity | ||
[InlineData("-1.7976931348623157E+308", 100_000)] // Min Negative Normal | ||
[InlineData("-3.1415926535897931", 1_000_000)] // Negative pi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about numbers with commas (thousands separator)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also - numbers with white space at the beginning/end. from https://docs.microsoft.com/en-us/dotnet/api/system.double.tryparse
In reply to: 219635010 [](ancestors = 219635010) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These few tests are already adding on 6min 20sec onto the total benchmark time (38 new tests at 10s each). I think there are a number of other scenarios that would be good to test, but the current Benchmark.NET setup does not make that very feasible, so I tried to cover a number of useful/interesting inputs for the most common string format. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
From a cursory look, I think some of your existing ones can be eliminated. For example:
Yes, these are important numbers: Similar comment for:
I think we could probably drop 2-3 from each of those blocks, and add at least 1 scenario with a comma in it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Talked with @eerhardt briefly offline. I definitely agree that we should add more tests, but I am waiting to hear back on if we have or plan to have a better story for "end to end" benchmarks (so that we can actually validate the average perf for a broad range of inputs, or for benchmarks that take longer than 1 second per outer iteration to run). The current tests where chosen because they have the broadest impact on the current algorithm (overall) and will be the most useful in validating some related changes going in. |
||
[InlineData("-2.7182818284590451", 1_000_000)] // Negative e | ||
[InlineData("-1", 1_000_000)] // Negative One | ||
[InlineData("-2.2250738585072014E-308", 100_000)] // Max Negative Normal | ||
[InlineData("-2.2250738585072009E-308", 100_000)] // Min Negative Subnormal | ||
[InlineData("-4.94065645841247E-324", 100_000)] // Max Negative Subnormal (Negative Epsilon) | ||
[InlineData("-0.0", 10_000_000)] // Negative Zero | ||
[InlineData("NaN", 10_000_000)] // NaN | ||
[InlineData("0", 10_000_000)] // Positive Zero | ||
[InlineData("4.94065645841247E-324", 100_000)] // Min Positive Subnormal (Positive Epsilon) | ||
[InlineData("2.2250738585072009E-308", 100_000)] // Max Positive Subnormal | ||
[InlineData("2.2250738585072014E-308", 100_000)] // Min Positive Normal | ||
[InlineData("1", 1_000_000)] // Positive One | ||
[InlineData("2.7182818284590451", 1_000_000)] // Positive e | ||
[InlineData("3.1415926535897931", 1_000_000)] // Positive pi | ||
[InlineData("1.7976931348623157E+308", 100_000)] // Max Positive Normal | ||
[InlineData("∞", 10_000_000)] // Positive Infinity | ||
public void DefaultTryParse(string input, int innerIterations) | ||
{ | ||
foreach (var iteration in Benchmark.Iterations) | ||
{ | ||
using (iteration.StartMeasurement()) | ||
{ | ||
for (int i = 0; i < innerIterations; i++) | ||
{ | ||
_bool = double.TryParse(input, out var result); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want (or would it be valuable) to test TryParse that takes a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
} | ||
} | ||
} | ||
|
||
[Benchmark] | ||
[InlineData("zh", double.NegativeInfinity, 10_000_000)] // Negative Infinity | ||
[InlineData("zh", double.MinValue, 100_000)] // Min Negative Normal | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how much it matters, but do we want numbers that are larger than
10
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That shouldn't matter as much. The current algorithm is mostly dependent on the length of the input string rather than the actual magnitude of its value.