Runtime: Provide common Task results for "true", "false", "empty", "null", etc

[davkean]

It's extremely common both in the framework itself and applications built on top of the framework to statically cache your own versions of Task with common results retrieved via Task.FromResult, similar to:

``` C#
///

/// Provides sentinel Tasks that represent commonly returned values.
///

internal class TaskResult
{
///

/// Represents a Task that's completed successfully with the result of .
///

public static readonly Task False = Task.FromResult(false);

    /// <summary>
    ///     Represents a Task that's completed successfully with the result of <see langword="true"/>.
    /// </summary>
    public static readonly Task<bool> True = Task.FromResult(true);
}

```

Just a few examples in 5 minutes of searching, are:

AsyncTaskCache in mscorlib
ADP in System.Data
AsyncHelper in System.Xml
TplExtensions in Microsoft.VisualStudio.Threading
TaskResult in ProjectSystem

Similar to what it did with Task.CompletedTask, the BCL should just provide these already cached values for common values, including true, false, empty string ("") and null.

[JonHanna]

If they're genuinely that common (I've done this myself in libraries but I'm not sure how common the need is generally) then it might make more sense for Task.FromResult(false) to just return a cached singleton, unless it would also be likely that other people were depending on different calls returning different instances, similarly to how Task.Delay(0) returns the same cached instance as Task.CompletedTask (and did so prior to Task.CompletedTask being created.

Of course the downside is an extra check on each call, but I think that should be cut out by the jitter if a typeof(T) == typeof(bool) test was part of the logic there.

[stephentoub]

When Task.FromResult was introduced, we went back and forth on whether it should always return a new task or whether it should use the same cache that async methods do, e.g. http://source.dot.net/#System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs,808, which covers the values being mentioned. We could certainly revisit that decision; I don't have a strong argument for keeping it the way it is, other than theoretical compat concerns for folks relying on reference equality, but that's dubious, and the potential perf impact for cases like Int32 where there is some small extra overhead for the case where there's a cache miss.

[stephentoub]

including true, false, empty string ("") and null

true, false, and null are all included in the aforementioned async cache. Adding empty string to that could add non-trivial overheads for when T == string.

[davkean]

I'm completely open to just make Task.FromResult return a cached value instead of explicit cached values. What was the concerns last time around this? The overhead of the check?

[bbowyersmyth]

Checking if a string == string.Empty can just be determined with reference equality. Extra branch and field load.
Whether that provides any advantages I don't know.

[davkean]

@bbowyersmyth Reference equality would only work if I actually pass string.Empty instead of ("").

[bbowyersmyth]

@davkean Same thing. Console.Write(ReferenceEquals("", string.Empty));

[davkean]

Ah, they must have changed this in JIT Blue? They weren't equal circa 4.0 timeframe (unless you were NGEN'D) - we had a bug in the BCL due to it.

[bbowyersmyth]

Clarification: They get turned into the same thing, CLR has length checks in the string constructors (not sure if the JIT does it as well), but it is not a guarantee on all platforms. Should be good enough if a lightweight cache check is needed seeing no caching is happening now.

[stephentoub]

Should be good enough

How common is it to return an empty string from a Task<string>-returning method? My assumption has been "not very common", but maybe I'm wrong?

[stephentoub]

Checking if a string == string.Empty can just be determined with reference equality. Extra branch and field load.

To be clear, though, this wouldn't just be for strings. The implementation gets specialized for value types, but all reference types share the same implementation... we'd essentially be adding another branch here:
https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs#L800
for every single reference type using this, just to handle the case of caching for an empty string... my assumption has been that returning empty strings are not nearly common enough to make that worthwhile, but maybe I'm wrong.

[davkean]

Can not speak across the board, but in my project it's common enough that I'm explicitly handling it - in one scenario (open solution) we have about 100 KB (of 38 MB of all Task) of these allocated. We are a project system, so strings (and empty strings) are used as return values for certain path APIs and the returns of reading values from MSBuild properties ("empty" == not present).