Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Several updates to ValueTask #16098

Merged
merged 4 commits into from
Jan 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ public ConfiguredValueTaskAwaiter GetAwaiter() =>

/// <summary>Provides an awaiter for a <see cref="ConfiguredValueTaskAwaitable{TResult}"/>.</summary>
[StructLayout(LayoutKind.Auto)]
public struct ConfiguredValueTaskAwaiter : ICriticalNotifyCompletion, IConfiguredValueTaskAwaiter
public readonly struct ConfiguredValueTaskAwaiter : ICriticalNotifyCompletion, IConfiguredValueTaskAwaiter
{
/// <summary>The value being awaited.</summary>
private ValueTask<TResult> _value; // Methods are called on this; avoid making it readonly so as to avoid unnecessary copies
private readonly ValueTask<TResult> _value;
/// <summary>The value to pass to ConfigureAwait.</summary>
internal readonly bool _continueOnCapturedContext;

Expand Down Expand Up @@ -74,7 +74,11 @@ public void UnsafeOnCompleted(Action continuation) =>

/// <summary>Gets the task underlying the incomplete <see cref="_value"/>.</summary>
/// <remarks>This method is used when awaiting and IsCompleted returned false; thus we expect the value task to be wrapping a non-null task.</remarks>
(Task task, bool continueOnCapturedContext) IConfiguredValueTaskAwaiter.GetTask() => (_value.AsTaskExpectNonNull(), _continueOnCapturedContext);
Copy link
Member

Choose a reason for hiding this comment

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

I like the change to not use ValueTuple here. It will improve .NET Core as well.

ValueTuples are pretty expensive types with large footprint, unfortunately. Not as bad as Linq, but bad enough.

Copy link
Member

Choose a reason for hiding this comment

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

This makes me sad

Task IConfiguredValueTaskAwaiter.GetTask(out bool continueOnCapturedContext)
{
continueOnCapturedContext = _continueOnCapturedContext;
return _value.AsTaskExpectNonNull();
}
}
}

Expand All @@ -83,6 +87,6 @@ public void UnsafeOnCompleted(Action continuation) =>
/// </summary>
internal interface IConfiguredValueTaskAwaiter
{
(Task task, bool continueOnCapturedContext) GetTask();
Task GetTask(out bool continueOnCapturedContext);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
namespace System.Runtime.CompilerServices
{
/// <summary>Provides an awaiter for a <see cref="ValueTask{TResult}"/>.</summary>
public struct ValueTaskAwaiter<TResult> : ICriticalNotifyCompletion, IValueTaskAwaiter
public readonly struct ValueTaskAwaiter<TResult> : ICriticalNotifyCompletion, IValueTaskAwaiter
{
/// <summary>The value being awaited.</summary>
private ValueTask<TResult> _value; // Methods are called on this; avoid making it readonly so as to avoid unnecessary copies
private readonly ValueTask<TResult> _value;

/// <summary>Initializes the awaiter.</summary>
/// <param name="value">The value to be awaited.</param>
Expand Down
51 changes: 28 additions & 23 deletions src/mscorlib/shared/System/Threading/Tasks/ValueTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.ComponentModel;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

Expand Down Expand Up @@ -77,7 +76,7 @@ public ValueTask(Task<TResult> task)
}

_task = task;
_result = default(TResult);
_result = default;
}

/// <summary>Returns the hash code for this instance.</summary>
Expand Down Expand Up @@ -114,7 +113,12 @@ public Task<TResult> AsTask() =>
// Return the task if we were constructed from one, otherwise manufacture one. We don't
// cache the generated task into _task as it would end up changing both equality comparison
// and the hash code we generate in GetHashCode.
_task ?? AsyncTaskMethodBuilder<TResult>.GetTaskForResult(_result);
_task ??
#if netstandard
Task.FromResult(_result);
#else
AsyncTaskMethodBuilder<TResult>.GetTaskForResult(_result);
#endif

internal Task<TResult> AsTaskExpectNonNull() =>
// Return the task if we were constructed from one, otherwise manufacture one.
Expand All @@ -123,13 +127,24 @@ internal Task<TResult> AsTaskExpectNonNull() =>
_task ?? GetTaskForResultNoInlining();

[MethodImpl(MethodImplOptions.NoInlining)]
private Task<TResult> GetTaskForResultNoInlining() => AsyncTaskMethodBuilder<TResult>.GetTaskForResult(_result);
private Task<TResult> GetTaskForResultNoInlining() =>
#if netstandard
Task.FromResult(_result);
#else
AsyncTaskMethodBuilder<TResult>.GetTaskForResult(_result);
#endif

/// <summary>Gets whether the <see cref="ValueTask{TResult}"/> represents a completed operation.</summary>
public bool IsCompleted => _task == null || _task.IsCompleted;

/// <summary>Gets whether the <see cref="ValueTask{TResult}"/> represents a successfully completed operation.</summary>
public bool IsCompletedSuccessfully => _task == null || _task.IsCompletedSuccessfully;
public bool IsCompletedSuccessfully =>
_task == null ||
#if netstandard
_task.Status == TaskStatus.RanToCompletion;
#else
_task.IsCompletedSuccessfully;
#endif

/// <summary>Gets whether the <see cref="ValueTask{TResult}"/> represents a failed operation.</summary>
public bool IsFaulted => _task != null && _task.IsFaulted;
Expand All @@ -153,26 +168,16 @@ public ConfiguredValueTaskAwaitable<TResult> ConfigureAwait(bool continueOnCaptu
/// <summary>Gets a string-representation of this <see cref="ValueTask{TResult}"/>.</summary>
public override string ToString()
{
if (_task != null)
if (IsCompletedSuccessfully)
{
return _task.IsCompletedSuccessfully && _task.Result != null ?
_task.Result.ToString() :
string.Empty;
TResult result = Result;
if (result != null)
{
return result.ToString();
}
}
else
{
return _result != null ?
_result.ToString() :
string.Empty;
}
}

// TODO https://github.com/dotnet/corefx/issues/22171:
// Remove CreateAsyncMethodBuilder once the C# compiler relies on the AsyncBuilder attribute.

/// <summary>Creates a method builder for use with an async method.</summary>
/// <returns>The created builder.</returns>
[EditorBrowsable(EditorBrowsableState.Never)] // intended only for compiler consumption
public static AsyncValueTaskMethodBuilder<TResult> CreateAsyncMethodBuilder() => AsyncValueTaskMethodBuilder<TResult>.Create();
return string.Empty;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,8 @@ public void AwaitUnsafeOnCompleted<TAwaiter, TStateMachine>(
}
else if ((null != (object)default(TAwaiter)) && (awaiter is IConfiguredValueTaskAwaiter))
{
(Task task, bool continueOnCapturedContext) t = ((IConfiguredValueTaskAwaiter)awaiter).GetTask();
TaskAwaiter.UnsafeOnCompletedInternal(t.task, box, t.continueOnCapturedContext);
Task t = ((IConfiguredValueTaskAwaiter)awaiter).GetTask(out bool continueOnCapturedContext);
TaskAwaiter.UnsafeOnCompletedInternal(t, box, continueOnCapturedContext);
}
// The awaiter isn't specially known. Fall back to doing a normal await.
else
Expand Down