Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid allocations from getting JsonOptions in Minimal #45359

Merged
merged 4 commits into from
Dec 1, 2022
Merged

Avoid allocations from getting JsonOptions in Minimal #45359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
077bf9c
Get JsonOptions once
BrennanConroy Nov 29, 2022
73a5103
read json
BrennanConroy Nov 30, 2022
c46f5b6
cache expression
BrennanConroy Nov 30, 2022
92e9f61
compile
BrennanConroy Nov 30, 2022
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
88 changes: 49 additions & 39 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
using System.Text.Json;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Http.Json;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Internal;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;

namespace Microsoft.AspNetCore.Http;
Expand Down Expand Up @@ -268,6 +270,7 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat

var serviceProvider = options?.ServiceProvider ?? options?.EndpointBuilder?.ApplicationServices ?? EmptyServiceProvider.Instance;
var endpointBuilder = options?.EndpointBuilder ?? new RDFEndpointBuilder(serviceProvider);
var jsonSerializerOptions = serviceProvider.GetService<IOptions<JsonOptions>>()?.Value.SerializerOptions;

var factoryContext = new RequestDelegateFactoryContext
{
Expand All @@ -279,6 +282,8 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat
DisableInferredFromBody = options?.DisableInferBodyFromParameters ?? false,
EndpointBuilder = endpointBuilder,
MetadataAlreadyInferred = metadataResult is not null,
JsonSerializerOptions = jsonSerializerOptions,
JsonSerializerOptionsExpression = Expression.Constant(jsonSerializerOptions, typeof(JsonSerializerOptions)),
};

return factoryContext;
Expand Down Expand Up @@ -361,7 +366,7 @@ private static IReadOnlyList<object> AsReadOnlyList(IList<object> metadata)

var responseWritingMethodCall = factoryContext.ParamCheckExpressions.Count > 0 ?
CreateParamCheckingResponseWritingMethodCall(returnType, factoryContext) :
AddResponseWritingToMethodCall(factoryContext.MethodCall, returnType);
AddResponseWritingToMethodCall(factoryContext.MethodCall, returnType, factoryContext);

if (factoryContext.UsingTempSourceString)
{
Expand Down Expand Up @@ -922,7 +927,7 @@ private static Expression CreateParamCheckingResponseWritingMethodCall(Type retu
Expression.IfThen(
WasParamCheckFailureExpr,
Expression.Assign(StatusCodeExpr, Expression.Constant(400))),
AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType)
AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType, factoryContext)
);

checkParamAndCallMethod[factoryContext.ParamCheckExpressions.Count] = checkWasParamCheckFailureWithFilters;
Expand All @@ -940,7 +945,7 @@ private static Expression CreateParamCheckingResponseWritingMethodCall(Type retu
Expression.Block(
Expression.Assign(StatusCodeExpr, Expression.Constant(400)),
CompletedTaskExpr),
AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType));
AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType, factoryContext));
checkParamAndCallMethod[factoryContext.ParamCheckExpressions.Count] = checkWasParamCheckFailure;
}

Expand Down Expand Up @@ -988,7 +993,7 @@ private static void PopulateBuiltInResponseTypeMetadata(Type returnType, Endpoin
}
}

private static Expression AddResponseWritingToMethodCall(Expression methodCall, Type returnType)
private static Expression AddResponseWritingToMethodCall(Expression methodCall, Type returnType, RequestDelegateFactoryContext factoryContext)
{
// Exact request delegate match
if (returnType == typeof(void))
Expand All @@ -997,19 +1002,19 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
}
else if (returnType == typeof(object))
{
return Expression.Call(ExecuteAwaitedReturnMethod, methodCall, HttpContextExpr);
return Expression.Call(ExecuteAwaitedReturnMethod, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression);
}
else if (returnType == typeof(ValueTask<object>))
{
return Expression.Call(ExecuteValueTaskOfObjectMethod,
methodCall,
HttpContextExpr);
HttpContextExpr, factoryContext.JsonSerializerOptionsExpression);
}
else if (returnType == typeof(Task<object>))
{
return Expression.Call(ExecuteTaskOfObjectMethod,
methodCall,
HttpContextExpr);
HttpContextExpr, factoryContext.JsonSerializerOptionsExpression);
}
else if (AwaitableInfo.IsTypeAwaitable(returnType, out _))
{
Expand Down Expand Up @@ -1048,7 +1053,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
return Expression.Call(
ExecuteTaskOfTMethod.MakeGenericMethod(typeArg),
methodCall,
HttpContextExpr);
HttpContextExpr,
factoryContext.JsonSerializerOptionsExpression);
}
}
else if (returnType.IsGenericType &&
Expand Down Expand Up @@ -1076,7 +1082,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
return Expression.Call(
ExecuteValueTaskOfTMethod.MakeGenericMethod(typeArg),
methodCall,
HttpContextExpr);
HttpContextExpr,
factoryContext.JsonSerializerOptionsExpression);
}
}
else
Expand Down Expand Up @@ -1105,11 +1112,11 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
else if (returnType.IsValueType)
{
var box = Expression.TypeAs(methodCall, typeof(object));
return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, box);
return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, box, factoryContext.JsonSerializerOptionsExpression);
}
else
{
return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, methodCall);
return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, methodCall, factoryContext.JsonSerializerOptionsExpression);
}
}

Expand Down Expand Up @@ -1190,7 +1197,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
parameterTypeName,
parameterName,
factoryContext.AllowEmptyRequestBody,
factoryContext.ThrowOnBadRequest);
factoryContext.ThrowOnBadRequest,
factoryContext.JsonSerializerOptions);

if (!successful)
{
Expand All @@ -1214,7 +1222,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
parameterTypeName,
parameterName,
factoryContext.AllowEmptyRequestBody,
factoryContext.ThrowOnBadRequest);
factoryContext.ThrowOnBadRequest,
factoryContext.JsonSerializerOptions);

if (!successful)
{
Expand All @@ -1231,7 +1240,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
string parameterTypeName,
string parameterName,
bool allowEmptyRequestBody,
bool throwOnBadRequest)
bool throwOnBadRequest,
JsonSerializerOptions? jsonSerializerOptions)
{
object? defaultBodyValue = null;

Expand All @@ -1253,7 +1263,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
}
try
{
bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType);
bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType, jsonSerializerOptions);
}
catch (IOException ex)
{
Expand Down Expand Up @@ -2043,37 +2053,37 @@ private static MemberInfo GetMemberInfo<T>(Expression<T> expr)
// if necessary and restart the cycle until we've reached a terminal state (unknown type).
// We currently don't handle Task<unknown> or ValueTask<unknown>. We can support this later if this
// ends up being a common scenario.
private static Task ExecuteValueTaskOfObject(ValueTask<object> valueTask, HttpContext httpContext)
private static Task ExecuteValueTaskOfObject(ValueTask<object> valueTask, HttpContext httpContext, JsonSerializerOptions? options)
{
static async Task ExecuteAwaited(ValueTask<object> valueTask, HttpContext httpContext)
static async Task ExecuteAwaited(ValueTask<object> valueTask, HttpContext httpContext, JsonSerializerOptions? options)
{
await ExecuteAwaitedReturn(await valueTask, httpContext);
await ExecuteAwaitedReturn(await valueTask, httpContext, options);
}

if (valueTask.IsCompletedSuccessfully)
{
return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext);
return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, options);
}

return ExecuteAwaited(valueTask, httpContext);
return ExecuteAwaited(valueTask, httpContext, options);
}

private static Task ExecuteTaskOfObject(Task<object> task, HttpContext httpContext)
private static Task ExecuteTaskOfObject(Task<object> task, HttpContext httpContext, JsonSerializerOptions? options)
{
static async Task ExecuteAwaited(Task<object> task, HttpContext httpContext)
static async Task ExecuteAwaited(Task<object> task, HttpContext httpContext, JsonSerializerOptions? options)
{
await ExecuteAwaitedReturn(await task, httpContext);
await ExecuteAwaitedReturn(await task, httpContext, options);
}

if (task.IsCompletedSuccessfully)
{
return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext);
return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, options);
}

return ExecuteAwaited(task, httpContext);
return ExecuteAwaited(task, httpContext, options);
}

private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext)
private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions? options)
{
// Terminal built ins
if (obj is IResult result)
Expand All @@ -2088,25 +2098,25 @@ private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext)
else
{
// Otherwise, we JSON serialize when we reach the terminal state
return WriteJsonResponse(httpContext.Response, obj);
return WriteJsonResponse(httpContext.Response, obj, options);
}
}

private static Task ExecuteTaskOfT<T>(Task<T> task, HttpContext httpContext)
private static Task ExecuteTaskOfT<T>(Task<T> task, HttpContext httpContext, JsonSerializerOptions? options)
{
EnsureRequestTaskNotNull(task);

static async Task ExecuteAwaited(Task<T> task, HttpContext httpContext)
static async Task ExecuteAwaited(Task<T> task, HttpContext httpContext, JsonSerializerOptions? options)
{
await WriteJsonResponse(httpContext.Response, await task);
await WriteJsonResponse(httpContext.Response, await task, options);
}

if (task.IsCompletedSuccessfully)
{
return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult());
return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options);
}

return ExecuteAwaited(task, httpContext);
return ExecuteAwaited(task, httpContext, options);
}

private static Task ExecuteTaskOfString(Task<string?> task, HttpContext httpContext)
Expand Down Expand Up @@ -2182,19 +2192,19 @@ static async Task ExecuteAwaited(ValueTask task)
return ExecuteAwaited(valueTask);
}

private static Task ExecuteValueTaskOfT<T>(ValueTask<T> task, HttpContext httpContext)
private static Task ExecuteValueTaskOfT<T>(ValueTask<T> task, HttpContext httpContext, JsonSerializerOptions? options)
{
static async Task ExecuteAwaited(ValueTask<T> task, HttpContext httpContext)
static async Task ExecuteAwaited(ValueTask<T> task, HttpContext httpContext, JsonSerializerOptions? options)
{
await WriteJsonResponse(httpContext.Response, await task);
await WriteJsonResponse(httpContext.Response, await task, options);
}

if (task.IsCompletedSuccessfully)
{
return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult());
return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options);
}

return ExecuteAwaited(task, httpContext);
return ExecuteAwaited(task, httpContext, options);
}

private static Task ExecuteValueTaskOfString(ValueTask<string?> task, HttpContext httpContext)
Expand Down Expand Up @@ -2241,13 +2251,13 @@ private static async Task ExecuteResultWriteResponse(IResult? result, HttpContex
await EnsureRequestResultNotNull(result).ExecuteAsync(httpContext);
}

private static Task WriteJsonResponse(HttpResponse response, object? value)
private static Task WriteJsonResponse(HttpResponse response, object? value, JsonSerializerOptions? options)
{
// Call WriteAsJsonAsync() with the runtime type to serialize the runtime type rather than the declared type
// and avoid source generators issues.
// https://github.com/dotnet/aspnetcore/issues/43894
// https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism
return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, value is null ? typeof(object) : value.GetType(), default);
return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, value is null ? typeof(object) : value.GetType(), options, default);

}

Expand Down
5 changes: 5 additions & 0 deletions src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Linq.Expressions;
using System.Reflection;
using System.Text.Json;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;

Expand Down Expand Up @@ -56,4 +57,8 @@ internal sealed class RequestDelegateFactoryContext
public bool FilterFactoriesHaveRunWithoutModifyingPerRequestBehavior { get; set; }

public List<ParameterInfo> Parameters { get; set; } = new();

// Grab these options upfront to avoid the per request DI scope that would be made otherwise to get the options when writing Json
public JsonSerializerOptions? JsonSerializerOptions { get; set; }
public required Expression JsonSerializerOptionsExpression { get; set; }
}