Mvc: ValueTask<T> response body is serializing ValueTask instead of T

[redknightlois]

I recently upgraded a CoreCLR 1.1 codebase to C# 7.0 and tried to change the output of async controllers to diminish allocation pressure from Task<T> to ValueTask<T>. However the output of MVC is serializing ValueTask<T> instead of T

The response was supposed to be:

[
  {
    "code": "code",
    "displayName": "Coding"
  }
]

But when changed to ValueTask it ends up being:

{
  "isCompleted": false,
  "isCompletedSuccessfully": false,
  "isFaulted": false,
  "isCanceled": false,
  "result": {
    "value": [
      {
        "code": "code",
        "displayName": "Coding"
      }
    ],
    "formatters": [],
    "contentTypes": [],
    "declaredType": null,
    "statusCode": 200
  }
}

[TsengSR]

What's your controller code?

Using ValueTask<T> unusual in Controller actions as you are unlikely to save any allocations at all. ValueTask<T> is very specific for hot path code, which may return a value in sync way (i.e. when reading it from cache) or when the return value is know in advanced instead of using return Task.FromResult(...)

The use cases are pretty rare and I can only assume you are not correctly understanding when and where to use ValueTask<T>.

One example is, when you read from a stream with a fixed buffer size which returns the number of bytes read. In all calls to read, the operation will read the same amount of bytes (size of the byte cache), except for the last read, which will be different. In this cases you can profit from ValueTask<T>. But with Task you can reduce the allocations in these scenarios too, by caching Task.FromResuult(buffer.Length) and returning it each time.

So I really don't see a usable case in ASP.NET Core Controllers for it

[redknightlois]

Hi @TsengSR,

I work on database engine optimization and we host it on top of kestrel + webapi with even custom routing code because stock implementation is not fast enough for our purposes. Also my daily job revolves around controlling allocations, optimization and microoptimization. We agree it is not common, but it can still happen.

But for simplicity, don't trust me, let's assume I don't know what I am doing. It is still a bug, output is not being correctly generated for a viable async supported scenario.

The controller code can be anything, just change Task for ValueTask and you will be able to observe the behavior.

[rynowak]

We haven't implemented support for this yet. We have plans to do it as part of https://github.com/aspnet/Mvc/issues/5570

[davidfowl]

@rynowak are you going to update the ObjectMethodExecutor to do this?

[rynowak]

Yes, that's the plan.

We should implement support for awaitables in general, not just the set of two known ones. I believe the F# version will require some custom code because it's not an awaitable

[SteveSandersonMS]

Done. Actions can now return arbitrary awaitables, which includes ValueTask<T>.

[rynowak]

🍺 👍 🔥