Consider using ValueTask<T> instead of Task<T>

[terrajobst]

Flag internal/private methods that returns Ts that won't be entirely cached (e.g. bool) and where every caller of the method only ever awaits its result directly.

Category: Performance

[jeffhandley]

Estimates:

• Analyzer: Large
• Fixer: Large

[weitzhandler]

The problem with ValueTask is that it's hard to be used in generics since they don't implement a shared interface, so if you make a static class with some generics, you'll have to write out all types if one of the types can't be inferred.
See xunit/xunit#1919 and #25654.

[Joe4evr]

@weitzhandler If the only thing you're concerned with is not having to write unnecessary type arguments, then dotnet/csharplang#1349 has you covered.

[carlossanlop]

that won't be entirely cached (e.g. bool)

@terrajobst can you please expand on this? What do you mean by "entirely cached"?

every caller of the method only ever awaits its result directly

Do you mean the analyzer should only flag cases where the await keyword is next to the invocation itself?

For example, if you have:

private Task<MyType> MyMethodAsync() { ... }

Do you mean the analyzer should only suggest changing to ValueTask<T> when all the invocations of the method look like this?:

MyType myType = await MyMethodAsync();

But if the analyzer finds at least one invocation that looks like this, then we don't suggest changing to ValueTask<T>?:

Task<MyType> myTask = MyMethodAsync();
await myTask;

[bartonjs]

Video

Generally seems good as proposed. A few notes from the discussion

• It should follow existing conventions for InternalsVisibleTo with internal members.
• It should probably ignore virtual members in the initial implementation, then add them in later.
• The general rule is "if any caller code would have to change, don't recommend this change"
• It only applies to methods with the async keyword.

Category: Performance
Level: Info