DateTime.DaysInMonth minor performance tweaks

[Charles113]

Hello, i did some performance testing and would propose changes to DateTime.DaysInMonth(int year, int month)

private static ReadOnlySpan<byte> DaysInMonth365 => new byte[] { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };
DaysInMonth365[month - 1];

Change to:

private static readonly byte[] DaysInMonth365 = new byte[] { 0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };
DaysInMonth365[month];

As the array is used as a lookup i would remove the -1 Operation, also i think DaysInMonth365 should be a field instead of a property.
I've noticed 5x Performance gains by doing lookups in a byte[] field instead of creating a new ReadOnlySpan every time through a property.

[stephentoub]

I've noticed 5x Performance gains by doing lookups in a byte[] field instead of creating a new ReadOnlySpan every time through a property.

Can you please share your benchmark?

[Charles113]

I've noticed 5x Performance gains by doing lookups in a byte[] field instead of creating a new ReadOnlySpan every time through a property.

Can you please share your benchmark?

Sure

    public static void Test()
    {
        var month = 2;

        var warmUp1 = DaysInMonth365[month - 1];
        var warmUp2 = DaysInMonth365_B[month];

        var sw = new Stopwatch();
        sw.Start();

        for (int i = 0; i < 1e8; i++)
        {
            var r1 = DaysInMonth365[month - 1];
        }

        // 950 ms on my machine
        var elapsedOld = sw.ElapsedMilliseconds;
 
        Console.WriteLine($"elapsedOld {elapsedOld}");
        Debug.WriteLine($"elapsedOld {elapsedOld}");

        sw.Restart();

        for (int i = 0; i < 1e8; i++)
        {
            var r1 = DaysInMonth365_B[month];
        }

        // 110 ms on my machine
        var elapsedNew = sw.ElapsedMilliseconds;

        Console.WriteLine($"elapsedNew {elapsedNew}");
        Debug.WriteLine($"elapsedNew {elapsedNew}");
    }

    private static ReadOnlySpan<byte> DaysInMonth365 => new byte[] { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };

    private static readonly byte[] DaysInMonth365_B = new byte[] { 0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };

Its almost 10 to 1 Ratio in debug mode.
But i also noticed only a 20% difference in release mode, so maybe it's optimized enough already.

[huoyaoyuan]

    private static ReadOnlySpan<byte> P => new byte[] { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };

    private static readonly byte[] F = new byte[] { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };

    public byte Property(int i) => P[i];
    public byte Field(int i) => F[i];

; Method Program:Property(int):ubyte:this
G_M000_IG01:                ;; offset=0000H
       4883EC28             sub      rsp, 40

G_M000_IG02:                ;; offset=0004H
       83FA0C               cmp      edx, 12
       7315                 jae      SHORT G_M000_IG04
       8BC2                 mov      eax, edx
       48BAF02A32617F020000 mov      rdx, 0x27F61322AF0
       0FB60410             movzx    rax, byte  ptr [rax+rdx]

G_M000_IG03:                ;; offset=0019H
       4883C428             add      rsp, 40
       C3                   ret      

G_M000_IG04:                ;; offset=001EH
       E8AD63C35F           call     CORINFO_HELP_RNGCHKFAIL
       CC                   int3    

; Method Program:Field(int):ubyte:this
G_M000_IG01:                ;; offset=0000H
       56                   push     rsi
       4883EC20             sub      rsp, 32
       8BF2                 mov      esi, edx

G_M000_IG02:                ;; offset=0007H
       48B9A8CF943DFE7F0000 mov      rcx, 0x7FFE3D94CFA8
       BA04000000           mov      edx, 4
       E805D1AE5F           call     CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE
       48B8381FC07A8B020000 mov      rax, 0x28B7AC01F38
       488B00               mov      rax, gword ptr [rax]
       3B7008               cmp      esi, dword ptr [rax+08H]
       730D                 jae      SHORT G_M000_IG04
       8BD6                 mov      edx, esi
       0FB6441010           movzx    rax, byte  ptr [rax+rdx+10H]

G_M000_IG03:                ;; offset=0034H
       4883C420             add      rsp, 32
       5E                   pop      rsi
       C3                   ret      

G_M000_IG04:                ;; offset=003AH
       E89163C15F           call     CORINFO_HELP_RNGCHKFAIL
       CC                   int3     

Codegen for field is worse than property. But it looks a bit weird for call CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE.

[huoyaoyuan]

A raw main is the most common wrong way to measure performance. You should use BenchmarkDotNet to get correct measurement.

On 7.0 I get this:

public class Program
{
    static void Main()
    {
        BenchmarkRunner.Run<Program>();
    }

    private static ReadOnlySpan<byte> P => new byte[] { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };

    private static readonly byte[] F = new byte[] { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };

    [Params(2)]
    public int Month { get; set; }

    [Benchmark(Baseline = true)]
    public byte Property() => P[Month];

    [Benchmark]
    public byte Field() => F[Month];
}

Method	Month	Mean	Error	StdDev	Ratio	RatioSD
Property	2	0.0033 ns	0.0022 ns	0.0018 ns	?	?
Field	2	0.2163 ns	0.0021 ns	0.0018 ns	?	?

Do we have constant folding for this case?

[huoyaoyuan]

    [MethodImpl(MethodImplOptions.NoInlining)]
    private int GetMonth() => 2;

    [Benchmark(Baseline = true)]
    public byte Property() => P[GetMonth()];

    [Benchmark]
    public byte Field() => F[GetMonth()];

Method	Mean	Error	StdDev	Ratio	RatioSD
Property	0.8890 ns	0.0118 ns	0.0110 ns	1.00	0.00
Field	1.1058 ns	0.0254 ns	0.0237 ns	1.24	0.02

After inlining and constant folding disabled, the field is 24% slower than property.

The runtime has special treatment for ReadOnlySpan, and the compiler has also special optimization for constant spans.

[ufcpp]

@Charles113
see dotnet/roslyn#24621 dotnet/csharplang#5295
https://github.com/dotnet/csharplang/blob/main/meetings/working-groups/collection-literals/CL-2023-08-03.md#blittable-data

[EgorBo]

The "property" definitely is not an issue. Adding a leading zero element to DaysInMonth365 and DaysInMonth366 to skip -1 operation makes sense if it leads to visible improvements?

[EgorBo]

Do we have constant folding for this case?

ReadOnlySpan<> case (we usually call them "RVA arrays") does support constant folding for const indices, but it should not be the case for your benchmark - BDN is not expected to pass the value directly (at least I hope so).

static readonly doesn't support constant folding because the actual array is still mutable so JIT is not allowed to do such foldings.

[ghost]

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

Issue Details

---

Hello, i did some performance testing and would propose changes to DateTime.DaysInMonth(int year, int month)

private static ReadOnlySpan<byte> DaysInMonth365 => new byte[] { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };
DaysInMonth365[month - 1];

Change to:

private static readonly byte[] DaysInMonth365 = new byte[] { 0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };
DaysInMonth365[month];

As the array is used as a lookup i would remove the -1 Operation, also i think DaysInMonth365 should be a field instead of a property.
I've noticed 5x Performance gains by doing lookups in a byte[] field instead of creating a new ReadOnlySpan every time through a property.

Author:	Charles113
Assignees:	-
Labels:	area-System.Runtime, tenet-performance, untriaged
Milestone:	-

[Neme12]

Its almost 10 to 1 Ratio in debug mode.

Try reversing the order of those two and you'll probably get the opposite result.

[Neme12]

Its almost 10 to 1 Ratio in debug mode.

I see, that's the problem. You should never benchmark debug mode.

[Charles113]

Try reversing the order of those two and you'll probably get the opposite result.

I did 6 release mode tests now, with different order, Fields always came out on top.
But i can't reproduce my tests with Benchmark dot net, so i just have to trust you guys that my stopwatch is broken.

100000000 x 0,000002281506 ms, Total 228,15 ms, prop
100000000 x 0,000001687245 ms, Total 168,72 ms, field
100000000 x 0,000001282946 ms, Total 128,29 ms, field run 2
100000000 x 0,000001724070 ms, Total 172,41 ms, prop run 2
100000000 x 0,000001661510 ms, Total 166,15 ms, prop run 3
100000000 x 0,000001117891 ms, Total 111,79 ms, field run 3

I guess after a while cpu cache kicks in...
I didn't know about the compiler optimizations for ReadOnlySpans, I'm less worried about the property now.
@ufcpp, @stephentoub dotnet/csharplang#5295 helped me to understand, thank you

[danmoseley]

Ok I'll close. Thanks anyway @Charles113 for reporting.