[release/7.0] [Android][libs] Introduce DateTimeOffset.Now temporary fast result

[github-actions]

Backport of #74459 to release/7.0

/cc @mdh1418

Customer Impact

Significantly reduces the performance impact of DateTimeOffset.Now at first call to an avg 21.818ms (Issue reported 277ms).

Testing

Tested locally using speedscope to ensure that there was a significant performance improvement. Also tested scenarios in which multiple DateTimeOffset.Now calls are made to ensure locks/background threads worked properly with resilience to races and deadlocking.

Testing locally with the dotnet/runtime Android sample with this program

        Console.WriteLine($"The current time is `{DateTimeOffset.Now}`");
        Thread.Sleep(2000);
        var temp = DateTimeOffset.Now;
        Console.WriteLine($"Now the current time is `{temp}`");

The average time of the first call to DateTimeOffset.Now was 21.818ms
The average time of the next call to DateTimeOffset.Now after the background thread finished was 27.821ms
The issue's reported timing of DateTimeOffset.Now was 277ms

Risk

Low, only impacts DateTimeOffset.Now on Android.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

[dotnet-issue-labeler]

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.

[carlossanlop]

@eerhardt or @lambdageek can we get a code review sign off?

[steveisok]

@marek-safar Can you please review / approve?

[stephentoub]

What prevents the race condition where two threads both read s_startNewBackgroundThread as true? Seems like it'd be safer to have a local that determines whether this thread is the one that gets to start it, and that local is set appropriately inside of the above lock.

[steveisok]

I believe we made a change based on feedback from @grendello. I don't recall the specifics as to why.

[grendello]

The feedback was about the previous state of the code, where the thread was started within the same lock that the thread then takes, which could end in a deadlock. The assumption here is that since .Start() doesn't block the current thread, the flag is set likely before the thread actually is started. Even in the unlikely situation when two threads would attempt to start the worker thread, the only harm would be a little time wasted inside the worker thread. This code could use a semaphore, for instance, but we felt that would be an overkill.

[stephentoub]

Even in the unlikely situation when two threads would attempt to start the worker thread, the only harm would be a little time wasted inside the worker thread

Calling Start on a thread that's already started will result in an exception. So the concern I'm raising above is that this could lead to reliability problems, not just performance ones.

[grendello]

It's a very unlikely event, but sure, it is better to wrap it in in try/catch then and ignore the potential exception. Performance was the main point of this PR, so it would be a shame to sacrifice it to handle a very unlikely case with semaphores or similar mechanisms.

[stephentoub]

Why store the thread at all? You can use s_startNewBackgroundThread or equivalent for all coordination between threads.

[grendello]

It was in response to review on the original PR, here.

[ghost]

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

Issue Details

---

Backport of #74459 to release/7.0

/cc @mdh1418

Customer Impact

Significantly reduces the performance impact of DateTimeOffset.Now at first call to an avg 21.818ms (Issue reported 277ms).

Testing

Tested locally using speedscope to ensure that there was a significant performance improvement. Also tested scenarios in which multiple DateTimeOffset.Now calls are made to ensure locks/background threads worked properly with resilience to races and deadlocking.

Testing locally with the dotnet/runtime Android sample with this program

        Console.WriteLine($"The current time is `{DateTimeOffset.Now}`");
        Thread.Sleep(2000);
        var temp = DateTimeOffset.Now;
        Console.WriteLine($"Now the current time is `{temp}`");

The average time of the first call to DateTimeOffset.Now was 21.818ms
The average time of the next call to DateTimeOffset.Now after the background thread finished was 27.821ms
The issue's reported timing of DateTimeOffset.Now was 277ms

Risk

Low, only impacts DateTimeOffset.Now on Android.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:	github-actions[bot]
Assignees:	-
Labels:	area-System.Runtime
Milestone:	7.0.0

[carlossanlop]

Adding the label until the feedback is resolved and CI confirmed to be unrelated. Please ping me when ready so I can merge it.

[steveisok]

@carlossanlop this should be good to go once CI finishes.

[stephentoub]

Thanks for addressing my feedback.

[stephentoub]

(We should make sure to keep main up to date with the changes made here.)