Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize DateTime constructors (DateToTicks and IsLeapYear) #46245

Closed
wants to merge 2 commits into from

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Dec 19, 2020

Just a nano-optimization to make some of the DateTime constructors a bit faster, e.g.:

[Benchmark]
public DateTime CreateDate_const()
{
    return new DateTime(2021, 5, 20);
}

[Benchmark]
[Arguments(2021, 5, 20)]
public DateTime CreateDate_var(int y, int m, int d)
{
    return new DateTime(y, m, d); // and other constructors with DateToTicks/IsLeapYear checks inside
}

/*

            |           Method |      Mean |
            |----------------- |----------:|
     master | CreateDate_const | 0.3712 ns |
     PR     | CreateDate_const | 0.2356 ns |
     master |   CreateDate_var | 2.6116 ns |
     PR     |   CreateDate_var | 2.1922 ns |

*/

Codegen diff for CreateDate_var: https://www.diffchecker.com/BOTFFqql (336 bytes -> 286 bytes), for arm64: https://www.diffchecker.com/H8xUCOtO (448 bytes -> 400 bytes)

Feel free to close if you think the hacks aren't worth it 🙂

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

return (year & 3) == 0 && ((year & 15) == 0 || (year % 25) != 0);
return (year & 3) == 0 && ((year & 15) == 0 ||
// TODO: optimize "(uint)year % 25 != 0" in JIT to produce:
(year * -1030792151 > 171798691));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

int y = year - 1;
int n = y * 365 + y / 4 - y / 100 + y / 400 + days[month - 1] + day - 1;
uint y = (uint)(year - 1);
uint n = y * 365 + y / 4 - y / 100 + y / 400 + (uint)days[month - 1] + (uint)day - 1;
return n * TicksPerDay;
}
Copy link
Member Author

@EgorBo EgorBo Dec 19, 2020

Choose a reason for hiding this comment

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

y is int but always is in the [1..9999] range so can be treated as unsigned (X /u C is faster than X / C) sharplab.io

@@ -635,8 +635,8 @@ private static long DateToTicks(int year, int month, int day)
ThrowHelper.ThrowArgumentOutOfRange_BadYearMonthDay();
}

int y = year - 1;
int n = y * 365 + y / 4 - y / 100 + y / 400 + days[month - 1] + day - 1;
uint y = (uint)(year - 1);
Copy link
Member

Choose a reason for hiding this comment

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

A similar optimization is part of #46245

@@ -1130,7 +1130,9 @@ public static bool IsLeapYear(int year)
{
ThrowHelper.ThrowArgumentOutOfRange_Year();
}
return (year & 3) == 0 && ((year & 15) == 0 || (year % 25) != 0);
return (year & 3) == 0 && ((year & 15) == 0 ||
// TODO: optimize "(uint)year % 25 != 0" in JIT to produce:
Copy link
Member

Choose a reason for hiding this comment

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

Yes, we should fix the JIT instead of sprinkling manually generated magic div and mod everywhere. I had the same comment in #45479 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

ah didn't notice that pr 🙂

@EgorBo EgorBo closed this Dec 19, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants