@jkotas this is just porting what we have done in the full framework. Also, I have created a commit for corert we can use when we are mirroring this change theretarekgh/corert@f79f7c8

could you please take a look at both and let me know if you have any comment? I have tested this on coreclr, corert and ProjectN native and Win32. I did the testing on Windows version which support leaps seconds with inserting some fake leap seconds to the system.

Is there going to be a matching PR with the tests in CoreFX?

Is there going to be a matching PR with the tests in CoreFX?

We already have tests for DateTime/DateTimeOffset exercise Now, UtcNow. Writing any test specific for testing with leap seconds will not have any effect because we'll have to run on a machine support leap seconds (i.e. RS5) and the system will have a leaps seconds which we don't have any so far. This will be interesting more when we get in the future a leap seconds.

We already have tests

There are two separate problems:

1. Where/how/when we have machines with the right config available in automation. I understand that this may be not easy to solve momentarily. I assume that you got the right machine yourself.
2. Tests that hit the new code on the machine with the right config. These tests should be failing before your change and passing after your change. This is the part that I think should be send to CoreFX.

Tests that hit the new code on the machine with the right config. These tests should be failing before your change and passing after your change. This is the part that I think should be send to CoreFX.

I'll write a test for that but I'll not expect this test will fail now and the possibility to fail in the future will be low. I understand your point and would be good to have it to catch if we would have a problem.

Do you have the number for performance hit that the methods likeDateTime.UtcNow will incur on the leap second aware systems with this change?

I think we had it in one of the offline conversations ... it would be useful to have the number on here too.

Looks good to me otherwise

Do you have the number for performance hit that the methods like DateTime.UtcNow will incur on the leap second aware systems with this change?

I don't have the exact numbers here and also I am using a VM for my leap seconds testing. but I have tested it and we always reporting the time on the same milliseconds boundary. Here example for what I was doing:

SYSTEMTIMEst=newSYSTEMTIME();GetSystemTime(refst);DateTimedt=DateTime.UtcNow;Console.WriteLine($"<{st.wHour:D2}:{st.wMinute:D2}:{st.wSecond:D2}.{st.wMillisecond:D2}> .... [{dt.Hour:D2}:{dt.Minute:D2}:{dt.Second:D2}.{dt.Millisecond:D2}]");

Note that we still reporting the precise time as we used to do even in leap seconds system. the only extra perf is just the time we use till we return. I can do the measurements later to get the exact numbers. note that we already have a perf tests for this too. anyway, let me know if you want to wait not merging the change till we do that or we case merge and do the measurements later. if we want to wait, I'll hold this changes till next year.

let me know if you want to wait not merging the change till we do that

I do not think you need to block on this.

test Windows_NT x64 Checked Innerloop Build and Test (Jit - TieredCompilation=0)

thanks@jkotas for your review and comments. I will wait preparing the changes for corert too and test it so we'll be ready when we merge. I'll let you know when I have it ready to take a quick look there too.

here is the delta changes in corert repo tootarekgh/corert@b503742 i have tested it with PN too. after reviewing the delta, I'll squash moth commits in corert to one so it will be easy to use.

Thanks a lot@jkotas for your review. I have updated the corert too and squashed the commits into one committarekgh/corert@389615a

I added a comment too on corert changetarekgh/corert@389615a#r31609074 may be we need to apply.

@Anipik I am noticing there is no corert mirroring PR opened for this one. Is it expected?

@tarekgh it is expected in the sense that there can be only one open mirror pr in a repo at any time.
And we have already have one in corert which hasnt yet mergeddotnet/corert#6663
After this PR has been merged the next pr for corert repo will contain this commit

Thanks@Anipik