Refactor some DateTime and TimeSpan formatting/parsing methods

[lilinus]

• Use FormattingHelpers.CountDigits.
• Use TimeSpanParse.Pow10 in DateTimeFormat.
• Change fraction variables from long to int.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

[danmoseley]

Are these sufficiently covered in dotnet/performance? Might be worth running appropriate benchmarks there.

[tarekgh]

_num = (int)(((uint)_num + power / 2) / power);

I am not sure if this dividing by 2 and dividing by power will be more performant than just calling Math.Round?
@tannergooding any opinion on that?

[huoyaoyuan]

constintMaxFractionDigits=7;[Params(123456789)]publicintNum{get;set;}[Params(10)]publicintTotalDigitsCount{get;set;}internalstaticintPow10UpToMaxFractionDigits(intpow){ReadOnlySpan<int>powersOfTen=[1,10,100,1000,10000,100000,1000000,10000000,];Debug.Assert(powersOfTen.Length==MaxFractionDigits+1);returnpowersOfTen[pow];}[Benchmark]publicintMathRound()=>(int)Math.Round((double)Num/Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits),MidpointRounding.AwayFromZero);[Benchmark]publicintIntegerRound(){uintpower=(uint)Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits);return(int)(((uint)Num+power/2)/power);}

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.8892 ns	0.0068 ns	0.0061 ns
IntegerRound	123456789	10	1.2103 ns	0.0044 ns	0.0034 ns

Interesting...I suppose floating point math could be much slower than integer.

[tarekgh]

I suggest reverting the code back to use Math.Round. It is even more readable and clear the intention of what the code does.

[xtqqczze]

Results on arm64 with the span bounds check removed:

BenchmarkDotNet v0.13.12, macOS Sonoma 14.4.1 (23E224) [Darwin 23.4.0]
Apple M2, 1 CPU, 8 logical and 8 physical cores
.NET SDK 8.0.204
[Host] : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD
DefaultJob : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.4477 ns	0.0105 ns	0.0093 ns
IntegerRound	123456789	10	0.1372 ns	0.0037 ns	0.0032 ns

[EgorBo]

Results on arm64 with the span bounds check removed:

Can you provide a public API benchmark that shows any noticeable perf difference? IMO, It doesn't make sense to benchmark internals.

[xtqqczze]

@EgorBo Not sure about real-world benchmarks, but with the suggested change code size (on X64) is 172 bytes vs is 271 bytes.

https://csharp.godbolt.org/z/YcMKW5fza

[tarekgh]

@lilinus is it possible you run your perf tests again mentioned in #101640 (comment) with @xtqqczze suggestion to get some idea about the perf of this change? Thanks!

I am talking about the change

uintpower=(uint)Pow10UpToMaxFractionDigits(totalDigitsCount-MaxFractionDigits);_num=(int)(((uint)_num+power/2)/power);

[lilinus]

Seems like uint division is slightly faster. I assume you want me to push back to that change so I will do it.

| Method | Job | Toolchain | arg | Mean | Error | StdDev | Median | Min | Max | Ratio | Allocated | Alloc Ratio |
|--------------- |----------- |---------------|------------------------- |---------:|--------:|--------:|---------:|---------:|---------:|------:|----------:|------------:|
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.000000001234567 | 215.0 ns | 0.90 ns | 0.84 ns | 215.0 ns | 213.3 ns | 216.7 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.000000001234567 | 154.5 ns | 0.43 ns | 0.36 ns | 154.4 ns | 154.1 ns | 155.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Mth.Round | 00:00:00.000000001234567 | 155.8 ns | 0.40 ns | 0.35 ns | 155.7 ns | 155.1 ns | 156.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.00001234567 | 156.1 ns | 0.48 ns | 0.43 ns | 155.8 ns | 155.6 ns | 157.0 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.00001234567 | 147.1 ns | 0.72 ns | 0.68 ns | 146.7 ns | 146.3 ns | 148.4 ns | 0.94 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Math.Round | 00:00:00.00001234567 | 151.3 ns | 0.53 ns | 0.50 ns | 151.4 ns | 150.2 ns | 151.9 ns | 0.97 | - | NA |

Benchmark source:

public static IEnumerable<object> Values()
{
yield return "00:00:00.00001234567";
yield return "00:00:00.000000001234567";
}
[Benchmark, ArgumentsSource(nameof(Values))]
public TimeSpan TimeSpan_Parse(string arg) => TimeSpan.Parse(arg, CultureInfo.InvariantCulture);

[tarekgh]

@lilinus looks you already did :-) I am fine with that as tt provides us with a smaller code size and a slightly faster performance as well.

One last ask, please add a comment above that line telling what is does to be easier for anyone reading the code understand what the code is doing.

@xtqqczze thanks for the suggestion!

[lilinus]

Comment added 👍

[tarekgh]

@lilinus thanks for helping with this change.

[tarekgh]

@lilinus could you please merge the latest change from main to this PR to be able to proceed?

[lilinus]

@tarekgh, done. Thanks for the feedback etc. I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

[tarekgh]

I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

Is it possible you can temporary disable the antivirus? Or add exclusion to the runtime and test folders to have the antivirus ignore them?

[lilinus]

Managed to get it working. Benchmarked some DateTime and TimeSpan parsing. Results are looking good IMO. Link to benchmarks: here

BenchmarkDotNet v0.13.13-nightly.20240311.145, Windows 10 (10.0.19044.3086/21H2/November2021Update)
AMD Ryzen 9 4900HS with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-preview.3.24204.13
[Host] : .NET 9.0.0 (9.0.24.17209), X64 RyuJIT AVX2
Job-GLLMPZ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-AQRKOX : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
PowerPlanMode=00000000-0000-0000-0000-000000000000 IterationTime=250ms MaxIterationCount=20
MinIterationCount=15 WarmupCount=1
| Method | Job | Toolchain | arg1 | arg2 | Mean | Error | StdDev | Median | Min | Max | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 78.58 ns | 1.200 ns | 1.123 ns | 78.43 ns | 76.52 ns | 80.90 ns | 1.00 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 64.23 ns | 0.889 ns | 0.831 ns | 64.38 ns | 62.55 ns | 65.61 ns | 0.82 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 72.58 ns | 0.264 ns | 0.234 ns | 72.57 ns | 72.30 ns | 73.05 ns | 1.00 | 0.00 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 63.92 ns | 0.693 ns | 0.614 ns | 63.78 ns | 62.92 ns | 64.95 ns | 0.88 | 0.01 | - | - | NA |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 213.02 ns | 0.465 ns | 0.388 ns | 212.95 ns | 212.32 ns | 213.69 ns | 1.00 | 0.00 | 0.0337 | 72 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 179.06 ns | 0.303 ns | 0.268 ns | 179.04 ns | 178.52 ns | 179.53 ns | 0.84 | 0.00 | 0.0343 | 72 B | 1.00 |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 146.01 ns | 0.579 ns | 0.542 ns | 145.84 ns | 145.43 ns | 147.21 ns | 1.00 | 0.01 | 0.0305 | 64 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 109.59 ns | 0.142 ns | 0.126 ns | 109.61 ns | 109.28 ns | 109.79 ns | 0.75 | 0.00 | 0.0304 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 177.14 ns | 0.506 ns | 0.473 ns | 177.02 ns | 176.46 ns | 178.08 ns | 1.00 | 0.00 | 0.0303 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 164.35 ns | 0.436 ns | 0.407 ns | 164.33 ns | 163.68 ns | 165.22 ns | 0.93 | 0.00 | 0.0301 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 184.79 ns | 0.532 ns | 0.497 ns | 184.62 ns | 184.17 ns | 185.89 ns | 1.00 | 0.00 | 0.0300 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 177.78 ns | 0.358 ns | 0.317 ns | 177.76 ns | 177.20 ns | 178.39 ns | 0.96 | 0.00 | 0.0305 | 64 B | 1.00 |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 225.67 ns | 0.378 ns | 0.335 ns | 225.61 ns | 225.21 ns | 226.24 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 202.20 ns | 0.250 ns | 0.209 ns | 202.22 ns | 201.87 ns | 202.51 ns | 0.90 | 0.00 | - | - | NA |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 219.44 ns | 0.367 ns | 0.325 ns | 219.45 ns | 218.83 ns | 220.02 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 218.17 ns | 0.361 ns | 0.320 ns | 218.18 ns | 217.31 ns | 218.65 ns | 0.99 | 0.00 | - | - | NA |

[lilinus]

• Use FormattingHelpers.CountDigits.
• Use TimeSpanParse.Pow10 in DateTimeFormat.
• Change fraction variables from long to int.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

[danmoseley]

Are these sufficiently covered in dotnet/performance? Might be worth running appropriate benchmarks there.

[tarekgh]

_num = (int)(((uint)_num + power / 2) / power);

I am not sure if this dividing by 2 and dividing by power will be more performant than just calling Math.Round?
@tannergooding any opinion on that?

[huoyaoyuan]

constintMaxFractionDigits=7;[Params(123456789)]publicintNum{get;set;}[Params(10)]publicintTotalDigitsCount{get;set;}internalstaticintPow10UpToMaxFractionDigits(intpow){ReadOnlySpan<int>powersOfTen=[1,10,100,1000,10000,100000,1000000,10000000,];Debug.Assert(powersOfTen.Length==MaxFractionDigits+1);returnpowersOfTen[pow];}[Benchmark]publicintMathRound()=>(int)Math.Round((double)Num/Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits),MidpointRounding.AwayFromZero);[Benchmark]publicintIntegerRound(){uintpower=(uint)Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits);return(int)(((uint)Num+power/2)/power);}

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.8892 ns	0.0068 ns	0.0061 ns
IntegerRound	123456789	10	1.2103 ns	0.0044 ns	0.0034 ns

Interesting...I suppose floating point math could be much slower than integer.

[tarekgh]

I suggest reverting the code back to use Math.Round. It is even more readable and clear the intention of what the code does.

[xtqqczze]

Results on arm64 with the span bounds check removed:

BenchmarkDotNet v0.13.12, macOS Sonoma 14.4.1 (23E224) [Darwin 23.4.0]
Apple M2, 1 CPU, 8 logical and 8 physical cores
.NET SDK 8.0.204
[Host] : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD
DefaultJob : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.4477 ns	0.0105 ns	0.0093 ns
IntegerRound	123456789	10	0.1372 ns	0.0037 ns	0.0032 ns

[EgorBo]

Results on arm64 with the span bounds check removed:

Can you provide a public API benchmark that shows any noticeable perf difference? IMO, It doesn't make sense to benchmark internals.

[xtqqczze]

@EgorBo Not sure about real-world benchmarks, but with the suggested change code size (on X64) is 172 bytes vs is 271 bytes.

https://csharp.godbolt.org/z/YcMKW5fza

[tarekgh]

@lilinus is it possible you run your perf tests again mentioned in #101640 (comment) with @xtqqczze suggestion to get some idea about the perf of this change? Thanks!

I am talking about the change

uintpower=(uint)Pow10UpToMaxFractionDigits(totalDigitsCount-MaxFractionDigits);_num=(int)(((uint)_num+power/2)/power);

[lilinus]

Seems like uint division is slightly faster. I assume you want me to push back to that change so I will do it.

| Method | Job | Toolchain | arg | Mean | Error | StdDev | Median | Min | Max | Ratio | Allocated | Alloc Ratio |
|--------------- |----------- |---------------|------------------------- |---------:|--------:|--------:|---------:|---------:|---------:|------:|----------:|------------:|
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.000000001234567 | 215.0 ns | 0.90 ns | 0.84 ns | 215.0 ns | 213.3 ns | 216.7 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.000000001234567 | 154.5 ns | 0.43 ns | 0.36 ns | 154.4 ns | 154.1 ns | 155.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Mth.Round | 00:00:00.000000001234567 | 155.8 ns | 0.40 ns | 0.35 ns | 155.7 ns | 155.1 ns | 156.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.00001234567 | 156.1 ns | 0.48 ns | 0.43 ns | 155.8 ns | 155.6 ns | 157.0 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.00001234567 | 147.1 ns | 0.72 ns | 0.68 ns | 146.7 ns | 146.3 ns | 148.4 ns | 0.94 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Math.Round | 00:00:00.00001234567 | 151.3 ns | 0.53 ns | 0.50 ns | 151.4 ns | 150.2 ns | 151.9 ns | 0.97 | - | NA |

Benchmark source:

public static IEnumerable<object> Values()
{
yield return "00:00:00.00001234567";
yield return "00:00:00.000000001234567";
}
[Benchmark, ArgumentsSource(nameof(Values))]
public TimeSpan TimeSpan_Parse(string arg) => TimeSpan.Parse(arg, CultureInfo.InvariantCulture);

[tarekgh]

@lilinus looks you already did :-) I am fine with that as tt provides us with a smaller code size and a slightly faster performance as well.

One last ask, please add a comment above that line telling what is does to be easier for anyone reading the code understand what the code is doing.

@xtqqczze thanks for the suggestion!

[lilinus]

Comment added 👍

[tarekgh]

@lilinus thanks for helping with this change.

[tarekgh]

@lilinus could you please merge the latest change from main to this PR to be able to proceed?

[lilinus]

@tarekgh, done. Thanks for the feedback etc. I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

[tarekgh]

I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

Is it possible you can temporary disable the antivirus? Or add exclusion to the runtime and test folders to have the antivirus ignore them?

[lilinus]

Managed to get it working. Benchmarked some DateTime and TimeSpan parsing. Results are looking good IMO. Link to benchmarks: here

BenchmarkDotNet v0.13.13-nightly.20240311.145, Windows 10 (10.0.19044.3086/21H2/November2021Update)
AMD Ryzen 9 4900HS with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-preview.3.24204.13
[Host] : .NET 9.0.0 (9.0.24.17209), X64 RyuJIT AVX2
Job-GLLMPZ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-AQRKOX : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
PowerPlanMode=00000000-0000-0000-0000-000000000000 IterationTime=250ms MaxIterationCount=20
MinIterationCount=15 WarmupCount=1
| Method | Job | Toolchain | arg1 | arg2 | Mean | Error | StdDev | Median | Min | Max | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 78.58 ns | 1.200 ns | 1.123 ns | 78.43 ns | 76.52 ns | 80.90 ns | 1.00 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 64.23 ns | 0.889 ns | 0.831 ns | 64.38 ns | 62.55 ns | 65.61 ns | 0.82 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 72.58 ns | 0.264 ns | 0.234 ns | 72.57 ns | 72.30 ns | 73.05 ns | 1.00 | 0.00 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 63.92 ns | 0.693 ns | 0.614 ns | 63.78 ns | 62.92 ns | 64.95 ns | 0.88 | 0.01 | - | - | NA |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 213.02 ns | 0.465 ns | 0.388 ns | 212.95 ns | 212.32 ns | 213.69 ns | 1.00 | 0.00 | 0.0337 | 72 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 179.06 ns | 0.303 ns | 0.268 ns | 179.04 ns | 178.52 ns | 179.53 ns | 0.84 | 0.00 | 0.0343 | 72 B | 1.00 |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 146.01 ns | 0.579 ns | 0.542 ns | 145.84 ns | 145.43 ns | 147.21 ns | 1.00 | 0.01 | 0.0305 | 64 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 109.59 ns | 0.142 ns | 0.126 ns | 109.61 ns | 109.28 ns | 109.79 ns | 0.75 | 0.00 | 0.0304 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 177.14 ns | 0.506 ns | 0.473 ns | 177.02 ns | 176.46 ns | 178.08 ns | 1.00 | 0.00 | 0.0303 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 164.35 ns | 0.436 ns | 0.407 ns | 164.33 ns | 163.68 ns | 165.22 ns | 0.93 | 0.00 | 0.0301 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 184.79 ns | 0.532 ns | 0.497 ns | 184.62 ns | 184.17 ns | 185.89 ns | 1.00 | 0.00 | 0.0300 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 177.78 ns | 0.358 ns | 0.317 ns | 177.76 ns | 177.20 ns | 178.39 ns | 0.96 | 0.00 | 0.0305 | 64 B | 1.00 |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 225.67 ns | 0.378 ns | 0.335 ns | 225.61 ns | 225.21 ns | 226.24 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 202.20 ns | 0.250 ns | 0.209 ns | 202.22 ns | 201.87 ns | 202.51 ns | 0.90 | 0.00 | - | - | NA |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 219.44 ns | 0.367 ns | 0.325 ns | 219.45 ns | 218.83 ns | 220.02 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 218.17 ns | 0.361 ns | 0.320 ns | 218.18 ns | 217.31 ns | 218.65 ns | 0.99 | 0.00 | - | - | NA |

[lilinus]

• Use FormattingHelpers.CountDigits.
• Use TimeSpanParse.Pow10 in DateTimeFormat.
• Change fraction variables from long to int.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

[danmoseley]

Are these sufficiently covered in dotnet/performance? Might be worth running appropriate benchmarks there.

[tarekgh]

_num = (int)(((uint)_num + power / 2) / power);

I am not sure if this dividing by 2 and dividing by power will be more performant than just calling Math.Round?
@tannergooding any opinion on that?

[huoyaoyuan]

constintMaxFractionDigits=7;[Params(123456789)]publicintNum{get;set;}[Params(10)]publicintTotalDigitsCount{get;set;}internalstaticintPow10UpToMaxFractionDigits(intpow){ReadOnlySpan<int>powersOfTen=[1,10,100,1000,10000,100000,1000000,10000000,];Debug.Assert(powersOfTen.Length==MaxFractionDigits+1);returnpowersOfTen[pow];}[Benchmark]publicintMathRound()=>(int)Math.Round((double)Num/Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits),MidpointRounding.AwayFromZero);[Benchmark]publicintIntegerRound(){uintpower=(uint)Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits);return(int)(((uint)Num+power/2)/power);}

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.8892 ns	0.0068 ns	0.0061 ns
IntegerRound	123456789	10	1.2103 ns	0.0044 ns	0.0034 ns

Interesting...I suppose floating point math could be much slower than integer.

[tarekgh]

I suggest reverting the code back to use Math.Round. It is even more readable and clear the intention of what the code does.

[xtqqczze]

Results on arm64 with the span bounds check removed:

BenchmarkDotNet v0.13.12, macOS Sonoma 14.4.1 (23E224) [Darwin 23.4.0]
Apple M2, 1 CPU, 8 logical and 8 physical cores
.NET SDK 8.0.204
[Host] : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD
DefaultJob : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.4477 ns	0.0105 ns	0.0093 ns
IntegerRound	123456789	10	0.1372 ns	0.0037 ns	0.0032 ns

[EgorBo]

Results on arm64 with the span bounds check removed:

Can you provide a public API benchmark that shows any noticeable perf difference? IMO, It doesn't make sense to benchmark internals.

[xtqqczze]

@EgorBo Not sure about real-world benchmarks, but with the suggested change code size (on X64) is 172 bytes vs is 271 bytes.

https://csharp.godbolt.org/z/YcMKW5fza

[tarekgh]

@lilinus is it possible you run your perf tests again mentioned in #101640 (comment) with @xtqqczze suggestion to get some idea about the perf of this change? Thanks!

I am talking about the change

uintpower=(uint)Pow10UpToMaxFractionDigits(totalDigitsCount-MaxFractionDigits);_num=(int)(((uint)_num+power/2)/power);

[lilinus]

Seems like uint division is slightly faster. I assume you want me to push back to that change so I will do it.

| Method | Job | Toolchain | arg | Mean | Error | StdDev | Median | Min | Max | Ratio | Allocated | Alloc Ratio |
|--------------- |----------- |---------------|------------------------- |---------:|--------:|--------:|---------:|---------:|---------:|------:|----------:|------------:|
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.000000001234567 | 215.0 ns | 0.90 ns | 0.84 ns | 215.0 ns | 213.3 ns | 216.7 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.000000001234567 | 154.5 ns | 0.43 ns | 0.36 ns | 154.4 ns | 154.1 ns | 155.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Mth.Round | 00:00:00.000000001234567 | 155.8 ns | 0.40 ns | 0.35 ns | 155.7 ns | 155.1 ns | 156.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.00001234567 | 156.1 ns | 0.48 ns | 0.43 ns | 155.8 ns | 155.6 ns | 157.0 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.00001234567 | 147.1 ns | 0.72 ns | 0.68 ns | 146.7 ns | 146.3 ns | 148.4 ns | 0.94 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Math.Round | 00:00:00.00001234567 | 151.3 ns | 0.53 ns | 0.50 ns | 151.4 ns | 150.2 ns | 151.9 ns | 0.97 | - | NA |

Benchmark source:

public static IEnumerable<object> Values()
{
yield return "00:00:00.00001234567";
yield return "00:00:00.000000001234567";
}
[Benchmark, ArgumentsSource(nameof(Values))]
public TimeSpan TimeSpan_Parse(string arg) => TimeSpan.Parse(arg, CultureInfo.InvariantCulture);

[tarekgh]

@lilinus looks you already did :-) I am fine with that as tt provides us with a smaller code size and a slightly faster performance as well.

One last ask, please add a comment above that line telling what is does to be easier for anyone reading the code understand what the code is doing.

@xtqqczze thanks for the suggestion!

[lilinus]

Comment added 👍

[tarekgh]

@lilinus thanks for helping with this change.

[tarekgh]

@lilinus could you please merge the latest change from main to this PR to be able to proceed?

[lilinus]

@tarekgh, done. Thanks for the feedback etc. I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

[tarekgh]

I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

Is it possible you can temporary disable the antivirus? Or add exclusion to the runtime and test folders to have the antivirus ignore them?

[lilinus]

Managed to get it working. Benchmarked some DateTime and TimeSpan parsing. Results are looking good IMO. Link to benchmarks: here

BenchmarkDotNet v0.13.13-nightly.20240311.145, Windows 10 (10.0.19044.3086/21H2/November2021Update)
AMD Ryzen 9 4900HS with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-preview.3.24204.13
[Host] : .NET 9.0.0 (9.0.24.17209), X64 RyuJIT AVX2
Job-GLLMPZ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-AQRKOX : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
PowerPlanMode=00000000-0000-0000-0000-000000000000 IterationTime=250ms MaxIterationCount=20
MinIterationCount=15 WarmupCount=1
| Method | Job | Toolchain | arg1 | arg2 | Mean | Error | StdDev | Median | Min | Max | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 78.58 ns | 1.200 ns | 1.123 ns | 78.43 ns | 76.52 ns | 80.90 ns | 1.00 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 64.23 ns | 0.889 ns | 0.831 ns | 64.38 ns | 62.55 ns | 65.61 ns | 0.82 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 72.58 ns | 0.264 ns | 0.234 ns | 72.57 ns | 72.30 ns | 73.05 ns | 1.00 | 0.00 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 63.92 ns | 0.693 ns | 0.614 ns | 63.78 ns | 62.92 ns | 64.95 ns | 0.88 | 0.01 | - | - | NA |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 213.02 ns | 0.465 ns | 0.388 ns | 212.95 ns | 212.32 ns | 213.69 ns | 1.00 | 0.00 | 0.0337 | 72 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 179.06 ns | 0.303 ns | 0.268 ns | 179.04 ns | 178.52 ns | 179.53 ns | 0.84 | 0.00 | 0.0343 | 72 B | 1.00 |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 146.01 ns | 0.579 ns | 0.542 ns | 145.84 ns | 145.43 ns | 147.21 ns | 1.00 | 0.01 | 0.0305 | 64 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 109.59 ns | 0.142 ns | 0.126 ns | 109.61 ns | 109.28 ns | 109.79 ns | 0.75 | 0.00 | 0.0304 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 177.14 ns | 0.506 ns | 0.473 ns | 177.02 ns | 176.46 ns | 178.08 ns | 1.00 | 0.00 | 0.0303 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 164.35 ns | 0.436 ns | 0.407 ns | 164.33 ns | 163.68 ns | 165.22 ns | 0.93 | 0.00 | 0.0301 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 184.79 ns | 0.532 ns | 0.497 ns | 184.62 ns | 184.17 ns | 185.89 ns | 1.00 | 0.00 | 0.0300 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 177.78 ns | 0.358 ns | 0.317 ns | 177.76 ns | 177.20 ns | 178.39 ns | 0.96 | 0.00 | 0.0305 | 64 B | 1.00 |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 225.67 ns | 0.378 ns | 0.335 ns | 225.61 ns | 225.21 ns | 226.24 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 202.20 ns | 0.250 ns | 0.209 ns | 202.22 ns | 201.87 ns | 202.51 ns | 0.90 | 0.00 | - | - | NA |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 219.44 ns | 0.367 ns | 0.325 ns | 219.45 ns | 218.83 ns | 220.02 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 218.17 ns | 0.361 ns | 0.320 ns | 218.18 ns | 217.31 ns | 218.65 ns | 0.99 | 0.00 | - | - | NA |

[lilinus]

• Use FormattingHelpers.CountDigits.
• Use TimeSpanParse.Pow10 in DateTimeFormat.
• Change fraction variables from long to int.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

[danmoseley]

Are these sufficiently covered in dotnet/performance? Might be worth running appropriate benchmarks there.

[tarekgh]

_num = (int)(((uint)_num + power / 2) / power);

I am not sure if this dividing by 2 and dividing by power will be more performant than just calling Math.Round?
@tannergooding any opinion on that?

[huoyaoyuan]

constintMaxFractionDigits=7;[Params(123456789)]publicintNum{get;set;}[Params(10)]publicintTotalDigitsCount{get;set;}internalstaticintPow10UpToMaxFractionDigits(intpow){ReadOnlySpan<int>powersOfTen=[1,10,100,1000,10000,100000,1000000,10000000,];Debug.Assert(powersOfTen.Length==MaxFractionDigits+1);returnpowersOfTen[pow];}[Benchmark]publicintMathRound()=>(int)Math.Round((double)Num/Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits),MidpointRounding.AwayFromZero);[Benchmark]publicintIntegerRound(){uintpower=(uint)Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits);return(int)(((uint)Num+power/2)/power);}

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.8892 ns	0.0068 ns	0.0061 ns
IntegerRound	123456789	10	1.2103 ns	0.0044 ns	0.0034 ns

Interesting...I suppose floating point math could be much slower than integer.

[tarekgh]

I suggest reverting the code back to use Math.Round. It is even more readable and clear the intention of what the code does.

[xtqqczze]

Results on arm64 with the span bounds check removed:

BenchmarkDotNet v0.13.12, macOS Sonoma 14.4.1 (23E224) [Darwin 23.4.0]
Apple M2, 1 CPU, 8 logical and 8 physical cores
.NET SDK 8.0.204
[Host] : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD
DefaultJob : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.4477 ns	0.0105 ns	0.0093 ns
IntegerRound	123456789	10	0.1372 ns	0.0037 ns	0.0032 ns

[EgorBo]

Results on arm64 with the span bounds check removed:

Can you provide a public API benchmark that shows any noticeable perf difference? IMO, It doesn't make sense to benchmark internals.

[xtqqczze]

@EgorBo Not sure about real-world benchmarks, but with the suggested change code size (on X64) is 172 bytes vs is 271 bytes.

https://csharp.godbolt.org/z/YcMKW5fza

[tarekgh]

@lilinus is it possible you run your perf tests again mentioned in #101640 (comment) with @xtqqczze suggestion to get some idea about the perf of this change? Thanks!

I am talking about the change

uintpower=(uint)Pow10UpToMaxFractionDigits(totalDigitsCount-MaxFractionDigits);_num=(int)(((uint)_num+power/2)/power);

[lilinus]

Seems like uint division is slightly faster. I assume you want me to push back to that change so I will do it.

| Method | Job | Toolchain | arg | Mean | Error | StdDev | Median | Min | Max | Ratio | Allocated | Alloc Ratio |
|--------------- |----------- |---------------|------------------------- |---------:|--------:|--------:|---------:|---------:|---------:|------:|----------:|------------:|
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.000000001234567 | 215.0 ns | 0.90 ns | 0.84 ns | 215.0 ns | 213.3 ns | 216.7 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.000000001234567 | 154.5 ns | 0.43 ns | 0.36 ns | 154.4 ns | 154.1 ns | 155.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Mth.Round | 00:00:00.000000001234567 | 155.8 ns | 0.40 ns | 0.35 ns | 155.7 ns | 155.1 ns | 156.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.00001234567 | 156.1 ns | 0.48 ns | 0.43 ns | 155.8 ns | 155.6 ns | 157.0 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.00001234567 | 147.1 ns | 0.72 ns | 0.68 ns | 146.7 ns | 146.3 ns | 148.4 ns | 0.94 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Math.Round | 00:00:00.00001234567 | 151.3 ns | 0.53 ns | 0.50 ns | 151.4 ns | 150.2 ns | 151.9 ns | 0.97 | - | NA |

Benchmark source:

public static IEnumerable<object> Values()
{
yield return "00:00:00.00001234567";
yield return "00:00:00.000000001234567";
}
[Benchmark, ArgumentsSource(nameof(Values))]
public TimeSpan TimeSpan_Parse(string arg) => TimeSpan.Parse(arg, CultureInfo.InvariantCulture);

[tarekgh]

@lilinus looks you already did :-) I am fine with that as tt provides us with a smaller code size and a slightly faster performance as well.

One last ask, please add a comment above that line telling what is does to be easier for anyone reading the code understand what the code is doing.

@xtqqczze thanks for the suggestion!

[lilinus]

Comment added 👍

[tarekgh]

@lilinus thanks for helping with this change.

[tarekgh]

@lilinus could you please merge the latest change from main to this PR to be able to proceed?

[lilinus]

@tarekgh, done. Thanks for the feedback etc. I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

[tarekgh]

I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

Is it possible you can temporary disable the antivirus? Or add exclusion to the runtime and test folders to have the antivirus ignore them?

[lilinus]

Managed to get it working. Benchmarked some DateTime and TimeSpan parsing. Results are looking good IMO. Link to benchmarks: here

BenchmarkDotNet v0.13.13-nightly.20240311.145, Windows 10 (10.0.19044.3086/21H2/November2021Update)
AMD Ryzen 9 4900HS with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-preview.3.24204.13
[Host] : .NET 9.0.0 (9.0.24.17209), X64 RyuJIT AVX2
Job-GLLMPZ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-AQRKOX : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
PowerPlanMode=00000000-0000-0000-0000-000000000000 IterationTime=250ms MaxIterationCount=20
MinIterationCount=15 WarmupCount=1
| Method | Job | Toolchain | arg1 | arg2 | Mean | Error | StdDev | Median | Min | Max | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 78.58 ns | 1.200 ns | 1.123 ns | 78.43 ns | 76.52 ns | 80.90 ns | 1.00 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 64.23 ns | 0.889 ns | 0.831 ns | 64.38 ns | 62.55 ns | 65.61 ns | 0.82 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 72.58 ns | 0.264 ns | 0.234 ns | 72.57 ns | 72.30 ns | 73.05 ns | 1.00 | 0.00 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 63.92 ns | 0.693 ns | 0.614 ns | 63.78 ns | 62.92 ns | 64.95 ns | 0.88 | 0.01 | - | - | NA |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 213.02 ns | 0.465 ns | 0.388 ns | 212.95 ns | 212.32 ns | 213.69 ns | 1.00 | 0.00 | 0.0337 | 72 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 179.06 ns | 0.303 ns | 0.268 ns | 179.04 ns | 178.52 ns | 179.53 ns | 0.84 | 0.00 | 0.0343 | 72 B | 1.00 |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 146.01 ns | 0.579 ns | 0.542 ns | 145.84 ns | 145.43 ns | 147.21 ns | 1.00 | 0.01 | 0.0305 | 64 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 109.59 ns | 0.142 ns | 0.126 ns | 109.61 ns | 109.28 ns | 109.79 ns | 0.75 | 0.00 | 0.0304 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 177.14 ns | 0.506 ns | 0.473 ns | 177.02 ns | 176.46 ns | 178.08 ns | 1.00 | 0.00 | 0.0303 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 164.35 ns | 0.436 ns | 0.407 ns | 164.33 ns | 163.68 ns | 165.22 ns | 0.93 | 0.00 | 0.0301 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 184.79 ns | 0.532 ns | 0.497 ns | 184.62 ns | 184.17 ns | 185.89 ns | 1.00 | 0.00 | 0.0300 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 177.78 ns | 0.358 ns | 0.317 ns | 177.76 ns | 177.20 ns | 178.39 ns | 0.96 | 0.00 | 0.0305 | 64 B | 1.00 |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 225.67 ns | 0.378 ns | 0.335 ns | 225.61 ns | 225.21 ns | 226.24 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 202.20 ns | 0.250 ns | 0.209 ns | 202.22 ns | 201.87 ns | 202.51 ns | 0.90 | 0.00 | - | - | NA |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 219.44 ns | 0.367 ns | 0.325 ns | 219.45 ns | 218.83 ns | 220.02 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 218.17 ns | 0.361 ns | 0.320 ns | 218.18 ns | 217.31 ns | 218.65 ns | 0.99 | 0.00 | - | - | NA |

[lilinus]

• Use FormattingHelpers.CountDigits.
• Use TimeSpanParse.Pow10 in DateTimeFormat.
• Change fraction variables from long to int.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

[danmoseley]

Are these sufficiently covered in dotnet/performance? Might be worth running appropriate benchmarks there.

[tarekgh]

_num = (int)(((uint)_num + power / 2) / power);

I am not sure if this dividing by 2 and dividing by power will be more performant than just calling Math.Round?
@tannergooding any opinion on that?

[huoyaoyuan]

constintMaxFractionDigits=7;[Params(123456789)]publicintNum{get;set;}[Params(10)]publicintTotalDigitsCount{get;set;}internalstaticintPow10UpToMaxFractionDigits(intpow){ReadOnlySpan<int>powersOfTen=[1,10,100,1000,10000,100000,1000000,10000000,];Debug.Assert(powersOfTen.Length==MaxFractionDigits+1);returnpowersOfTen[pow];}[Benchmark]publicintMathRound()=>(int)Math.Round((double)Num/Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits),MidpointRounding.AwayFromZero);[Benchmark]publicintIntegerRound(){uintpower=(uint)Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits);return(int)(((uint)Num+power/2)/power);}

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.8892 ns	0.0068 ns	0.0061 ns
IntegerRound	123456789	10	1.2103 ns	0.0044 ns	0.0034 ns

Interesting...I suppose floating point math could be much slower than integer.

[tarekgh]

I suggest reverting the code back to use Math.Round. It is even more readable and clear the intention of what the code does.

[xtqqczze]

Results on arm64 with the span bounds check removed:

BenchmarkDotNet v0.13.12, macOS Sonoma 14.4.1 (23E224) [Darwin 23.4.0]
Apple M2, 1 CPU, 8 logical and 8 physical cores
.NET SDK 8.0.204
[Host] : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD
DefaultJob : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.4477 ns	0.0105 ns	0.0093 ns
IntegerRound	123456789	10	0.1372 ns	0.0037 ns	0.0032 ns

[EgorBo]

Results on arm64 with the span bounds check removed:

Can you provide a public API benchmark that shows any noticeable perf difference? IMO, It doesn't make sense to benchmark internals.

[xtqqczze]

@EgorBo Not sure about real-world benchmarks, but with the suggested change code size (on X64) is 172 bytes vs is 271 bytes.

https://csharp.godbolt.org/z/YcMKW5fza

[tarekgh]

@lilinus is it possible you run your perf tests again mentioned in #101640 (comment) with @xtqqczze suggestion to get some idea about the perf of this change? Thanks!

I am talking about the change

uintpower=(uint)Pow10UpToMaxFractionDigits(totalDigitsCount-MaxFractionDigits);_num=(int)(((uint)_num+power/2)/power);

[lilinus]

Seems like uint division is slightly faster. I assume you want me to push back to that change so I will do it.

| Method | Job | Toolchain | arg | Mean | Error | StdDev | Median | Min | Max | Ratio | Allocated | Alloc Ratio |
|--------------- |----------- |---------------|------------------------- |---------:|--------:|--------:|---------:|---------:|---------:|------:|----------:|------------:|
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.000000001234567 | 215.0 ns | 0.90 ns | 0.84 ns | 215.0 ns | 213.3 ns | 216.7 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.000000001234567 | 154.5 ns | 0.43 ns | 0.36 ns | 154.4 ns | 154.1 ns | 155.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Mth.Round | 00:00:00.000000001234567 | 155.8 ns | 0.40 ns | 0.35 ns | 155.7 ns | 155.1 ns | 156.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.00001234567 | 156.1 ns | 0.48 ns | 0.43 ns | 155.8 ns | 155.6 ns | 157.0 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.00001234567 | 147.1 ns | 0.72 ns | 0.68 ns | 146.7 ns | 146.3 ns | 148.4 ns | 0.94 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Math.Round | 00:00:00.00001234567 | 151.3 ns | 0.53 ns | 0.50 ns | 151.4 ns | 150.2 ns | 151.9 ns | 0.97 | - | NA |

Benchmark source:

public static IEnumerable<object> Values()
{
yield return "00:00:00.00001234567";
yield return "00:00:00.000000001234567";
}
[Benchmark, ArgumentsSource(nameof(Values))]
public TimeSpan TimeSpan_Parse(string arg) => TimeSpan.Parse(arg, CultureInfo.InvariantCulture);

[tarekgh]

@lilinus looks you already did :-) I am fine with that as tt provides us with a smaller code size and a slightly faster performance as well.

One last ask, please add a comment above that line telling what is does to be easier for anyone reading the code understand what the code is doing.

@xtqqczze thanks for the suggestion!

[lilinus]

Comment added 👍

[tarekgh]

@lilinus thanks for helping with this change.

[tarekgh]

@lilinus could you please merge the latest change from main to this PR to be able to proceed?

[lilinus]

@tarekgh, done. Thanks for the feedback etc. I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

[tarekgh]

I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

Is it possible you can temporary disable the antivirus? Or add exclusion to the runtime and test folders to have the antivirus ignore them?

[lilinus]

Managed to get it working. Benchmarked some DateTime and TimeSpan parsing. Results are looking good IMO. Link to benchmarks: here

BenchmarkDotNet v0.13.13-nightly.20240311.145, Windows 10 (10.0.19044.3086/21H2/November2021Update)
AMD Ryzen 9 4900HS with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-preview.3.24204.13
[Host] : .NET 9.0.0 (9.0.24.17209), X64 RyuJIT AVX2
Job-GLLMPZ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-AQRKOX : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
PowerPlanMode=00000000-0000-0000-0000-000000000000 IterationTime=250ms MaxIterationCount=20
MinIterationCount=15 WarmupCount=1
| Method | Job | Toolchain | arg1 | arg2 | Mean | Error | StdDev | Median | Min | Max | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 78.58 ns | 1.200 ns | 1.123 ns | 78.43 ns | 76.52 ns | 80.90 ns | 1.00 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 64.23 ns | 0.889 ns | 0.831 ns | 64.38 ns | 62.55 ns | 65.61 ns | 0.82 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 72.58 ns | 0.264 ns | 0.234 ns | 72.57 ns | 72.30 ns | 73.05 ns | 1.00 | 0.00 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 63.92 ns | 0.693 ns | 0.614 ns | 63.78 ns | 62.92 ns | 64.95 ns | 0.88 | 0.01 | - | - | NA |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 213.02 ns | 0.465 ns | 0.388 ns | 212.95 ns | 212.32 ns | 213.69 ns | 1.00 | 0.00 | 0.0337 | 72 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 179.06 ns | 0.303 ns | 0.268 ns | 179.04 ns | 178.52 ns | 179.53 ns | 0.84 | 0.00 | 0.0343 | 72 B | 1.00 |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 146.01 ns | 0.579 ns | 0.542 ns | 145.84 ns | 145.43 ns | 147.21 ns | 1.00 | 0.01 | 0.0305 | 64 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 109.59 ns | 0.142 ns | 0.126 ns | 109.61 ns | 109.28 ns | 109.79 ns | 0.75 | 0.00 | 0.0304 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 177.14 ns | 0.506 ns | 0.473 ns | 177.02 ns | 176.46 ns | 178.08 ns | 1.00 | 0.00 | 0.0303 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 164.35 ns | 0.436 ns | 0.407 ns | 164.33 ns | 163.68 ns | 165.22 ns | 0.93 | 0.00 | 0.0301 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 184.79 ns | 0.532 ns | 0.497 ns | 184.62 ns | 184.17 ns | 185.89 ns | 1.00 | 0.00 | 0.0300 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 177.78 ns | 0.358 ns | 0.317 ns | 177.76 ns | 177.20 ns | 178.39 ns | 0.96 | 0.00 | 0.0305 | 64 B | 1.00 |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 225.67 ns | 0.378 ns | 0.335 ns | 225.61 ns | 225.21 ns | 226.24 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 202.20 ns | 0.250 ns | 0.209 ns | 202.22 ns | 201.87 ns | 202.51 ns | 0.90 | 0.00 | - | - | NA |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 219.44 ns | 0.367 ns | 0.325 ns | 219.45 ns | 218.83 ns | 220.02 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 218.17 ns | 0.361 ns | 0.320 ns | 218.18 ns | 217.31 ns | 218.65 ns | 0.99 | 0.00 | - | - | NA |

[lilinus]

• Use FormattingHelpers.CountDigits.
• Use TimeSpanParse.Pow10 in DateTimeFormat.
• Change fraction variables from long to int.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

[danmoseley]

Are these sufficiently covered in dotnet/performance? Might be worth running appropriate benchmarks there.

[tarekgh]

_num = (int)(((uint)_num + power / 2) / power);

I am not sure if this dividing by 2 and dividing by power will be more performant than just calling Math.Round?
@tannergooding any opinion on that?

[huoyaoyuan]

constintMaxFractionDigits=7;[Params(123456789)]publicintNum{get;set;}[Params(10)]publicintTotalDigitsCount{get;set;}internalstaticintPow10UpToMaxFractionDigits(intpow){ReadOnlySpan<int>powersOfTen=[1,10,100,1000,10000,100000,1000000,10000000,];Debug.Assert(powersOfTen.Length==MaxFractionDigits+1);returnpowersOfTen[pow];}[Benchmark]publicintMathRound()=>(int)Math.Round((double)Num/Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits),MidpointRounding.AwayFromZero);[Benchmark]publicintIntegerRound(){uintpower=(uint)Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits);return(int)(((uint)Num+power/2)/power);}

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.8892 ns	0.0068 ns	0.0061 ns
IntegerRound	123456789	10	1.2103 ns	0.0044 ns	0.0034 ns

Interesting...I suppose floating point math could be much slower than integer.

[tarekgh]

I suggest reverting the code back to use Math.Round. It is even more readable and clear the intention of what the code does.

[xtqqczze]

Results on arm64 with the span bounds check removed:

BenchmarkDotNet v0.13.12, macOS Sonoma 14.4.1 (23E224) [Darwin 23.4.0]
Apple M2, 1 CPU, 8 logical and 8 physical cores
.NET SDK 8.0.204
[Host] : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD
DefaultJob : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.4477 ns	0.0105 ns	0.0093 ns
IntegerRound	123456789	10	0.1372 ns	0.0037 ns	0.0032 ns

[EgorBo]

Results on arm64 with the span bounds check removed:

Can you provide a public API benchmark that shows any noticeable perf difference? IMO, It doesn't make sense to benchmark internals.

[xtqqczze]

@EgorBo Not sure about real-world benchmarks, but with the suggested change code size (on X64) is 172 bytes vs is 271 bytes.

https://csharp.godbolt.org/z/YcMKW5fza

[tarekgh]

@lilinus is it possible you run your perf tests again mentioned in #101640 (comment) with @xtqqczze suggestion to get some idea about the perf of this change? Thanks!

I am talking about the change

uintpower=(uint)Pow10UpToMaxFractionDigits(totalDigitsCount-MaxFractionDigits);_num=(int)(((uint)_num+power/2)/power);

[lilinus]

Seems like uint division is slightly faster. I assume you want me to push back to that change so I will do it.

| Method | Job | Toolchain | arg | Mean | Error | StdDev | Median | Min | Max | Ratio | Allocated | Alloc Ratio |
|--------------- |----------- |---------------|------------------------- |---------:|--------:|--------:|---------:|---------:|---------:|------:|----------:|------------:|
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.000000001234567 | 215.0 ns | 0.90 ns | 0.84 ns | 215.0 ns | 213.3 ns | 216.7 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.000000001234567 | 154.5 ns | 0.43 ns | 0.36 ns | 154.4 ns | 154.1 ns | 155.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Mth.Round | 00:00:00.000000001234567 | 155.8 ns | 0.40 ns | 0.35 ns | 155.7 ns | 155.1 ns | 156.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.00001234567 | 156.1 ns | 0.48 ns | 0.43 ns | 155.8 ns | 155.6 ns | 157.0 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.00001234567 | 147.1 ns | 0.72 ns | 0.68 ns | 146.7 ns | 146.3 ns | 148.4 ns | 0.94 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Math.Round | 00:00:00.00001234567 | 151.3 ns | 0.53 ns | 0.50 ns | 151.4 ns | 150.2 ns | 151.9 ns | 0.97 | - | NA |

Benchmark source:

public static IEnumerable<object> Values()
{
yield return "00:00:00.00001234567";
yield return "00:00:00.000000001234567";
}
[Benchmark, ArgumentsSource(nameof(Values))]
public TimeSpan TimeSpan_Parse(string arg) => TimeSpan.Parse(arg, CultureInfo.InvariantCulture);

[tarekgh]

@lilinus looks you already did :-) I am fine with that as tt provides us with a smaller code size and a slightly faster performance as well.

One last ask, please add a comment above that line telling what is does to be easier for anyone reading the code understand what the code is doing.

@xtqqczze thanks for the suggestion!

[lilinus]

Comment added 👍

[tarekgh]

@lilinus thanks for helping with this change.

[tarekgh]

@lilinus could you please merge the latest change from main to this PR to be able to proceed?

[lilinus]

@tarekgh, done. Thanks for the feedback etc. I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

[tarekgh]

I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

Is it possible you can temporary disable the antivirus? Or add exclusion to the runtime and test folders to have the antivirus ignore them?

[lilinus]

Managed to get it working. Benchmarked some DateTime and TimeSpan parsing. Results are looking good IMO. Link to benchmarks: here

BenchmarkDotNet v0.13.13-nightly.20240311.145, Windows 10 (10.0.19044.3086/21H2/November2021Update)
AMD Ryzen 9 4900HS with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-preview.3.24204.13
[Host] : .NET 9.0.0 (9.0.24.17209), X64 RyuJIT AVX2
Job-GLLMPZ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-AQRKOX : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
PowerPlanMode=00000000-0000-0000-0000-000000000000 IterationTime=250ms MaxIterationCount=20
MinIterationCount=15 WarmupCount=1
| Method | Job | Toolchain | arg1 | arg2 | Mean | Error | StdDev | Median | Min | Max | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 78.58 ns | 1.200 ns | 1.123 ns | 78.43 ns | 76.52 ns | 80.90 ns | 1.00 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 64.23 ns | 0.889 ns | 0.831 ns | 64.38 ns | 62.55 ns | 65.61 ns | 0.82 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 72.58 ns | 0.264 ns | 0.234 ns | 72.57 ns | 72.30 ns | 73.05 ns | 1.00 | 0.00 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 63.92 ns | 0.693 ns | 0.614 ns | 63.78 ns | 62.92 ns | 64.95 ns | 0.88 | 0.01 | - | - | NA |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 213.02 ns | 0.465 ns | 0.388 ns | 212.95 ns | 212.32 ns | 213.69 ns | 1.00 | 0.00 | 0.0337 | 72 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 179.06 ns | 0.303 ns | 0.268 ns | 179.04 ns | 178.52 ns | 179.53 ns | 0.84 | 0.00 | 0.0343 | 72 B | 1.00 |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 146.01 ns | 0.579 ns | 0.542 ns | 145.84 ns | 145.43 ns | 147.21 ns | 1.00 | 0.01 | 0.0305 | 64 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 109.59 ns | 0.142 ns | 0.126 ns | 109.61 ns | 109.28 ns | 109.79 ns | 0.75 | 0.00 | 0.0304 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 177.14 ns | 0.506 ns | 0.473 ns | 177.02 ns | 176.46 ns | 178.08 ns | 1.00 | 0.00 | 0.0303 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 164.35 ns | 0.436 ns | 0.407 ns | 164.33 ns | 163.68 ns | 165.22 ns | 0.93 | 0.00 | 0.0301 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 184.79 ns | 0.532 ns | 0.497 ns | 184.62 ns | 184.17 ns | 185.89 ns | 1.00 | 0.00 | 0.0300 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 177.78 ns | 0.358 ns | 0.317 ns | 177.76 ns | 177.20 ns | 178.39 ns | 0.96 | 0.00 | 0.0305 | 64 B | 1.00 |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 225.67 ns | 0.378 ns | 0.335 ns | 225.61 ns | 225.21 ns | 226.24 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 202.20 ns | 0.250 ns | 0.209 ns | 202.22 ns | 201.87 ns | 202.51 ns | 0.90 | 0.00 | - | - | NA |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 219.44 ns | 0.367 ns | 0.325 ns | 219.45 ns | 218.83 ns | 220.02 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 218.17 ns | 0.361 ns | 0.320 ns | 218.18 ns | 217.31 ns | 218.65 ns | 0.99 | 0.00 | - | - | NA |

[lilinus]

• Use FormattingHelpers.CountDigits.
• Use TimeSpanParse.Pow10 in DateTimeFormat.
• Change fraction variables from long to int.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

[danmoseley]

Are these sufficiently covered in dotnet/performance? Might be worth running appropriate benchmarks there.

[tarekgh]

_num = (int)(((uint)_num + power / 2) / power);

I am not sure if this dividing by 2 and dividing by power will be more performant than just calling Math.Round?
@tannergooding any opinion on that?

[huoyaoyuan]

constintMaxFractionDigits=7;[Params(123456789)]publicintNum{get;set;}[Params(10)]publicintTotalDigitsCount{get;set;}internalstaticintPow10UpToMaxFractionDigits(intpow){ReadOnlySpan<int>powersOfTen=[1,10,100,1000,10000,100000,1000000,10000000,];Debug.Assert(powersOfTen.Length==MaxFractionDigits+1);returnpowersOfTen[pow];}[Benchmark]publicintMathRound()=>(int)Math.Round((double)Num/Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits),MidpointRounding.AwayFromZero);[Benchmark]publicintIntegerRound(){uintpower=(uint)Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits);return(int)(((uint)Num+power/2)/power);}

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.8892 ns	0.0068 ns	0.0061 ns
IntegerRound	123456789	10	1.2103 ns	0.0044 ns	0.0034 ns

Interesting...I suppose floating point math could be much slower than integer.

[tarekgh]

I suggest reverting the code back to use Math.Round. It is even more readable and clear the intention of what the code does.

[xtqqczze]

Results on arm64 with the span bounds check removed:

BenchmarkDotNet v0.13.12, macOS Sonoma 14.4.1 (23E224) [Darwin 23.4.0]
Apple M2, 1 CPU, 8 logical and 8 physical cores
.NET SDK 8.0.204
[Host] : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD
DefaultJob : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.4477 ns	0.0105 ns	0.0093 ns
IntegerRound	123456789	10	0.1372 ns	0.0037 ns	0.0032 ns

[EgorBo]

Results on arm64 with the span bounds check removed:

Can you provide a public API benchmark that shows any noticeable perf difference? IMO, It doesn't make sense to benchmark internals.

[xtqqczze]

@EgorBo Not sure about real-world benchmarks, but with the suggested change code size (on X64) is 172 bytes vs is 271 bytes.

https://csharp.godbolt.org/z/YcMKW5fza

[tarekgh]

@lilinus is it possible you run your perf tests again mentioned in #101640 (comment) with @xtqqczze suggestion to get some idea about the perf of this change? Thanks!

I am talking about the change

uintpower=(uint)Pow10UpToMaxFractionDigits(totalDigitsCount-MaxFractionDigits);_num=(int)(((uint)_num+power/2)/power);

[lilinus]

Seems like uint division is slightly faster. I assume you want me to push back to that change so I will do it.

| Method | Job | Toolchain | arg | Mean | Error | StdDev | Median | Min | Max | Ratio | Allocated | Alloc Ratio |
|--------------- |----------- |---------------|------------------------- |---------:|--------:|--------:|---------:|---------:|---------:|------:|----------:|------------:|
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.000000001234567 | 215.0 ns | 0.90 ns | 0.84 ns | 215.0 ns | 213.3 ns | 216.7 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.000000001234567 | 154.5 ns | 0.43 ns | 0.36 ns | 154.4 ns | 154.1 ns | 155.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Mth.Round | 00:00:00.000000001234567 | 155.8 ns | 0.40 ns | 0.35 ns | 155.7 ns | 155.1 ns | 156.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.00001234567 | 156.1 ns | 0.48 ns | 0.43 ns | 155.8 ns | 155.6 ns | 157.0 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.00001234567 | 147.1 ns | 0.72 ns | 0.68 ns | 146.7 ns | 146.3 ns | 148.4 ns | 0.94 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Math.Round | 00:00:00.00001234567 | 151.3 ns | 0.53 ns | 0.50 ns | 151.4 ns | 150.2 ns | 151.9 ns | 0.97 | - | NA |

Benchmark source:

public static IEnumerable<object> Values()
{
yield return "00:00:00.00001234567";
yield return "00:00:00.000000001234567";
}
[Benchmark, ArgumentsSource(nameof(Values))]
public TimeSpan TimeSpan_Parse(string arg) => TimeSpan.Parse(arg, CultureInfo.InvariantCulture);

[tarekgh]

@lilinus looks you already did :-) I am fine with that as tt provides us with a smaller code size and a slightly faster performance as well.

One last ask, please add a comment above that line telling what is does to be easier for anyone reading the code understand what the code is doing.

@xtqqczze thanks for the suggestion!

[lilinus]

Comment added 👍

[tarekgh]

@lilinus thanks for helping with this change.

[tarekgh]

@lilinus could you please merge the latest change from main to this PR to be able to proceed?

[lilinus]

@tarekgh, done. Thanks for the feedback etc. I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

[tarekgh]

I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

Is it possible you can temporary disable the antivirus? Or add exclusion to the runtime and test folders to have the antivirus ignore them?

[lilinus]

Managed to get it working. Benchmarked some DateTime and TimeSpan parsing. Results are looking good IMO. Link to benchmarks: here

BenchmarkDotNet v0.13.13-nightly.20240311.145, Windows 10 (10.0.19044.3086/21H2/November2021Update)
AMD Ryzen 9 4900HS with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-preview.3.24204.13
[Host] : .NET 9.0.0 (9.0.24.17209), X64 RyuJIT AVX2
Job-GLLMPZ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-AQRKOX : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
PowerPlanMode=00000000-0000-0000-0000-000000000000 IterationTime=250ms MaxIterationCount=20
MinIterationCount=15 WarmupCount=1
| Method | Job | Toolchain | arg1 | arg2 | Mean | Error | StdDev | Median | Min | Max | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 78.58 ns | 1.200 ns | 1.123 ns | 78.43 ns | 76.52 ns | 80.90 ns | 1.00 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 64.23 ns | 0.889 ns | 0.831 ns | 64.38 ns | 62.55 ns | 65.61 ns | 0.82 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 72.58 ns | 0.264 ns | 0.234 ns | 72.57 ns | 72.30 ns | 73.05 ns | 1.00 | 0.00 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 63.92 ns | 0.693 ns | 0.614 ns | 63.78 ns | 62.92 ns | 64.95 ns | 0.88 | 0.01 | - | - | NA |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 213.02 ns | 0.465 ns | 0.388 ns | 212.95 ns | 212.32 ns | 213.69 ns | 1.00 | 0.00 | 0.0337 | 72 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 179.06 ns | 0.303 ns | 0.268 ns | 179.04 ns | 178.52 ns | 179.53 ns | 0.84 | 0.00 | 0.0343 | 72 B | 1.00 |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 146.01 ns | 0.579 ns | 0.542 ns | 145.84 ns | 145.43 ns | 147.21 ns | 1.00 | 0.01 | 0.0305 | 64 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 109.59 ns | 0.142 ns | 0.126 ns | 109.61 ns | 109.28 ns | 109.79 ns | 0.75 | 0.00 | 0.0304 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 177.14 ns | 0.506 ns | 0.473 ns | 177.02 ns | 176.46 ns | 178.08 ns | 1.00 | 0.00 | 0.0303 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 164.35 ns | 0.436 ns | 0.407 ns | 164.33 ns | 163.68 ns | 165.22 ns | 0.93 | 0.00 | 0.0301 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 184.79 ns | 0.532 ns | 0.497 ns | 184.62 ns | 184.17 ns | 185.89 ns | 1.00 | 0.00 | 0.0300 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 177.78 ns | 0.358 ns | 0.317 ns | 177.76 ns | 177.20 ns | 178.39 ns | 0.96 | 0.00 | 0.0305 | 64 B | 1.00 |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 225.67 ns | 0.378 ns | 0.335 ns | 225.61 ns | 225.21 ns | 226.24 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 202.20 ns | 0.250 ns | 0.209 ns | 202.22 ns | 201.87 ns | 202.51 ns | 0.90 | 0.00 | - | - | NA |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 219.44 ns | 0.367 ns | 0.325 ns | 219.45 ns | 218.83 ns | 220.02 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 218.17 ns | 0.361 ns | 0.320 ns | 218.18 ns | 217.31 ns | 218.65 ns | 0.99 | 0.00 | - | - | NA |

[lilinus]

• Use FormattingHelpers.CountDigits.
• Use TimeSpanParse.Pow10 in DateTimeFormat.
• Change fraction variables from long to int.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

[danmoseley]

Are these sufficiently covered in dotnet/performance? Might be worth running appropriate benchmarks there.

[tarekgh]

_num = (int)(((uint)_num + power / 2) / power);

I am not sure if this dividing by 2 and dividing by power will be more performant than just calling Math.Round?
@tannergooding any opinion on that?

[huoyaoyuan]

constintMaxFractionDigits=7;[Params(123456789)]publicintNum{get;set;}[Params(10)]publicintTotalDigitsCount{get;set;}internalstaticintPow10UpToMaxFractionDigits(intpow){ReadOnlySpan<int>powersOfTen=[1,10,100,1000,10000,100000,1000000,10000000,];Debug.Assert(powersOfTen.Length==MaxFractionDigits+1);returnpowersOfTen[pow];}[Benchmark]publicintMathRound()=>(int)Math.Round((double)Num/Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits),MidpointRounding.AwayFromZero);[Benchmark]publicintIntegerRound(){uintpower=(uint)Pow10UpToMaxFractionDigits(TotalDigitsCount-MaxFractionDigits);return(int)(((uint)Num+power/2)/power);}

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.8892 ns	0.0068 ns	0.0061 ns
IntegerRound	123456789	10	1.2103 ns	0.0044 ns	0.0034 ns

Interesting...I suppose floating point math could be much slower than integer.

[tarekgh]

I suggest reverting the code back to use Math.Round. It is even more readable and clear the intention of what the code does.

[xtqqczze]

Results on arm64 with the span bounds check removed:

BenchmarkDotNet v0.13.12, macOS Sonoma 14.4.1 (23E224) [Darwin 23.4.0]
Apple M2, 1 CPU, 8 logical and 8 physical cores
.NET SDK 8.0.204
[Host] : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD
DefaultJob : .NET 8.0.4 (8.0.424.16909), Arm64 RyuJIT AdvSIMD

Method	Num	TotalDigitsCount	Mean	Error	StdDev
MathRound	123456789	10	0.4477 ns	0.0105 ns	0.0093 ns
IntegerRound	123456789	10	0.1372 ns	0.0037 ns	0.0032 ns

[EgorBo]

Results on arm64 with the span bounds check removed:

Can you provide a public API benchmark that shows any noticeable perf difference? IMO, It doesn't make sense to benchmark internals.

[xtqqczze]

@EgorBo Not sure about real-world benchmarks, but with the suggested change code size (on X64) is 172 bytes vs is 271 bytes.

https://csharp.godbolt.org/z/YcMKW5fza

[tarekgh]

@lilinus is it possible you run your perf tests again mentioned in #101640 (comment) with @xtqqczze suggestion to get some idea about the perf of this change? Thanks!

I am talking about the change

uintpower=(uint)Pow10UpToMaxFractionDigits(totalDigitsCount-MaxFractionDigits);_num=(int)(((uint)_num+power/2)/power);

[lilinus]

Seems like uint division is slightly faster. I assume you want me to push back to that change so I will do it.

| Method | Job | Toolchain | arg | Mean | Error | StdDev | Median | Min | Max | Ratio | Allocated | Alloc Ratio |
|--------------- |----------- |---------------|------------------------- |---------:|--------:|--------:|---------:|---------:|---------:|------:|----------:|------------:|
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.000000001234567 | 215.0 ns | 0.90 ns | 0.84 ns | 215.0 ns | 213.3 ns | 216.7 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.000000001234567 | 154.5 ns | 0.43 ns | 0.36 ns | 154.4 ns | 154.1 ns | 155.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Mth.Round | 00:00:00.000000001234567 | 155.8 ns | 0.40 ns | 0.35 ns | 155.7 ns | 155.1 ns | 156.4 ns | 0.72 | - | NA |
| TimeSpan_Parse | Job-PGUVPC | main | 00:00:00.00001234567 | 156.1 ns | 0.48 ns | 0.43 ns | 155.8 ns | 155.6 ns | 157.0 ns | 1.00 | - | NA |
| TimeSpan_Parse | Job-AKPZDD | uint division | 00:00:00.00001234567 | 147.1 ns | 0.72 ns | 0.68 ns | 146.7 ns | 146.3 ns | 148.4 ns | 0.94 | - | NA |
| TimeSpan_Parse | Job-RERBPQ | Math.Round | 00:00:00.00001234567 | 151.3 ns | 0.53 ns | 0.50 ns | 151.4 ns | 150.2 ns | 151.9 ns | 0.97 | - | NA |

Benchmark source:

public static IEnumerable<object> Values()
{
yield return "00:00:00.00001234567";
yield return "00:00:00.000000001234567";
}
[Benchmark, ArgumentsSource(nameof(Values))]
public TimeSpan TimeSpan_Parse(string arg) => TimeSpan.Parse(arg, CultureInfo.InvariantCulture);

[tarekgh]

@lilinus looks you already did :-) I am fine with that as tt provides us with a smaller code size and a slightly faster performance as well.

One last ask, please add a comment above that line telling what is does to be easier for anyone reading the code understand what the code is doing.

@xtqqczze thanks for the suggestion!

[lilinus]

Comment added 👍

[tarekgh]

@lilinus thanks for helping with this change.

[tarekgh]

@lilinus could you please merge the latest change from main to this PR to be able to proceed?

[lilinus]

@tarekgh, done. Thanks for the feedback etc. I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

[tarekgh]

I also tried benchmarking the changes but had trouble with antivirus stopping the runtime.

Is it possible you can temporary disable the antivirus? Or add exclusion to the runtime and test folders to have the antivirus ignore them?

[lilinus]

Managed to get it working. Benchmarked some DateTime and TimeSpan parsing. Results are looking good IMO. Link to benchmarks: here

BenchmarkDotNet v0.13.13-nightly.20240311.145, Windows 10 (10.0.19044.3086/21H2/November2021Update)
AMD Ryzen 9 4900HS with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-preview.3.24204.13
[Host] : .NET 9.0.0 (9.0.24.17209), X64 RyuJIT AVX2
Job-GLLMPZ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
Job-AQRKOX : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
PowerPlanMode=00000000-0000-0000-0000-000000000000 IterationTime=250ms MaxIterationCount=20
MinIterationCount=15 WarmupCount=1
| Method | Job | Toolchain | arg1 | arg2 | Mean | Error | StdDev | Median | Min | Max | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 78.58 ns | 1.200 ns | 1.123 ns | 78.43 ns | 76.52 ns | 80.90 ns | 1.00 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.123 | d\\.hh\\.mm\\.ss\\.FFFFF | 64.23 ns | 0.889 ns | 0.831 ns | 64.38 ns | 62.55 ns | 65.61 ns | 0.82 | 0.02 | - | - | NA |
| TimeSpan_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 72.58 ns | 0.264 ns | 0.234 ns | 72.57 ns | 72.30 ns | 73.05 ns | 1.00 | 0.00 | - | - | NA |
| TimeSpan_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 0.00.00.00.12345 | d\\.hh\\.mm\\.ss\\.fffff | 63.92 ns | 0.693 ns | 0.614 ns | 63.78 ns | 62.92 ns | 64.95 ns | 0.88 | 0.01 | - | - | NA |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 213.02 ns | 0.465 ns | 0.388 ns | 212.95 ns | 212.32 ns | 213.69 ns | 1.00 | 0.00 | 0.0337 | 72 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fff | 179.06 ns | 0.303 ns | 0.268 ns | 179.04 ns | 178.52 ns | 179.53 ns | 0.84 | 0.00 | 0.0343 | 72 B | 1.00 |
| DateTime_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 146.01 ns | 0.579 ns | 0.542 ns | 145.84 ns | 145.43 ns | 147.21 ns | 1.00 | 0.01 | 0.0305 | 64 B | 1.00 |
| DateTime_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 01/01/0001 00:00:12 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 109.59 ns | 0.142 ns | 0.126 ns | 109.61 ns | 109.28 ns | 109.79 ns | 0.75 | 0.00 | 0.0304 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 177.14 ns | 0.506 ns | 0.473 ns | 177.02 ns | 176.46 ns | 178.08 ns | 1.00 | 0.00 | 0.0303 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.fffffff | 164.35 ns | 0.436 ns | 0.407 ns | 164.33 ns | 163.68 ns | 165.22 ns | 0.93 | 0.00 | 0.0301 | 64 B | 1.00 |
| TimeSpan_Format | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 184.79 ns | 0.532 ns | 0.497 ns | 184.62 ns | 184.17 ns | 185.89 ns | 1.00 | 0.00 | 0.0300 | 64 B | 1.00 |
| TimeSpan_Format | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 00:00:12.3456789 | d\\.hh\\.mm\\.ss\\.FFFFFFF | 177.78 ns | 0.358 ns | 0.317 ns | 177.76 ns | 177.20 ns | 178.39 ns | 0.96 | 0.00 | 0.0305 | 64 B | 1.00 |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 225.67 ns | 0.378 ns | 0.335 ns | 225.61 ns | 225.21 ns | 226.24 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.123 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.FFFFF | 202.20 ns | 0.250 ns | 0.209 ns | 202.22 ns | 201.87 ns | 202.51 ns | 0.90 | 0.00 | - | - | NA |
| DateTime_Parse | Job-GLLMPZ | \dnmain\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 219.44 ns | 0.367 ns | 0.325 ns | 219.45 ns | 218.83 ns | 220.02 ns | 1.00 | 0.00 | - | - | NA |
| DateTime_Parse | Job-AQRKOX | \dnrt\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\CoreRun.exe | 2024-05-02.21.40.42.12345 | yyyy\\-MM\\-dd\\.HH\\.mm\\.ss\\.fffff | 218.17 ns | 0.361 ns | 0.320 ns | 218.18 ns | 217.31 ns | 218.65 ns | 0.99 | 0.00 | - | - | NA |