Skip to content

Commit

Permalink
Fixed Pipeline Caching Issues.
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelstaib authored May 1, 2023
2 parents de2e6b1 + b030d4b commit dc10c4b
Show file tree
Hide file tree
Showing 49 changed files with 590 additions and 254 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
using HotChocolate.Types;
using HotChocolate.Validation;
using static System.Linq.Expressions.Expression;
using static HotChocolate.Execution.Properties.Resources;

namespace HotChocolate.Execution.Pipeline.Complexity;

internal sealed class ComplexityAnalyzerCompilerVisitor : TypeDocumentValidatorVisitor
internal sealed class ComplexityAnalyzerCompiler : TypeDocumentValidatorVisitor
{
private static readonly MethodInfo _getService =
typeof(IServiceProvider).GetMethod("GetService")!;
Expand All @@ -23,18 +24,15 @@ internal sealed class ComplexityAnalyzerCompilerVisitor : TypeDocumentValidatorV
private readonly ParameterExpression _services =
Parameter(typeof(IServiceProvider), "services");

public ComplexityAnalyzerCompilerVisitor(ComplexityAnalyzerSettings settings)
public ComplexityAnalyzerCompiler(ComplexityAnalyzerSettings settings)
{
_settings = Constant(settings, typeof(ComplexityAnalyzerSettings));
}

protected override ISyntaxVisitorAction Enter(
DocumentNode node,
IDocumentValidatorContext context)
{
context.List.Push(new List<OperationComplexityAnalyzer>());
return base.Enter(node, context);
}
=> throw new InvalidOperationException(ComplexityAnalyzerCompiler_Enter_OnlyOperations);

protected override ISyntaxVisitorAction Enter(
OperationDefinitionNode node,
Expand All @@ -49,14 +47,14 @@ protected override ISyntaxVisitorAction Leave(
IDocumentValidatorContext context)
{
var expressions = (List<Expression>)context.List.Pop()!;
var analyzers = (List<OperationComplexityAnalyzer>)context.List.Peek()!;

analyzers.Add(new OperationComplexityAnalyzer(
node,
Lambda<ComplexityAnalyzerDelegate>(
Combine(expressions),
_services,
_variables).Compile()));

context.List.Push(
new OperationComplexityAnalyzer(
node,
Lambda<ComplexityAnalyzerDelegate>(
Combine(expressions),
_services,
_variables).Compile()));

return base.Leave(node, context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,11 @@ public async ValueTask InvokeAsync(IRequestContext context)

if (operationId is null)
{
operationId = CreateOperationId(
context.DocumentId,
context.Request.OperationName);
operationId = context.CreateCacheId(context.DocumentId, context.Request.OperationName);
context.OperationId = operationId;
}

var cacheId = context.CreateCacheId(operationId);

if (_operationCache.TryGetOperation(cacheId, out var operation))
if (_operationCache.TryGetOperation(operationId, out var operation))
{
context.Operation = operation;
addToCache = false;
Expand All @@ -61,7 +57,7 @@ context.DocumentId is not null &&
context.Document is not null &&
context.IsValidDocument)
{
_operationCache.TryAddOperation(cacheId, context.Operation);
_operationCache.TryAddOperation(operationId, context.Operation);
_diagnosticEvents.AddedOperationToCache(context);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using HotChocolate.Execution.Caching;
using HotChocolate.Execution.Options;
Expand All @@ -19,7 +18,7 @@ internal sealed class OperationComplexityMiddleware
private readonly DocumentValidatorContextPool _contextPool;
private readonly ComplexityAnalyzerSettings _settings;
private readonly IComplexityAnalyzerCache _cache;
private readonly ComplexityAnalyzerCompilerVisitor _compiler;
private readonly ComplexityAnalyzerCompiler _compiler;
private readonly VariableCoercionHelper _coercionHelper;

public OperationComplexityMiddleware(
Expand All @@ -29,18 +28,22 @@ public OperationComplexityMiddleware(
IComplexityAnalyzerCache cache,
VariableCoercionHelper coercionHelper)
{
if (options == null)
{
throw new ArgumentNullException(nameof(options));
}

_next = next ??
throw new ArgumentNullException(nameof(next));
_contextPool = contextPool ??
throw new ArgumentNullException(nameof(contextPool));
_settings = options?.Complexity ??
throw new ArgumentNullException(nameof(options));
_settings = options.Complexity;
_cache = cache ??
throw new ArgumentNullException(nameof(cache));
_coercionHelper = coercionHelper ??
throw new ArgumentNullException(nameof(coercionHelper));

_compiler = new ComplexityAnalyzerCompilerVisitor(_settings);
_compiler = new ComplexityAnalyzerCompiler(_settings);
}

public async ValueTask InvokeAsync(IRequestContext context)
Expand All @@ -59,15 +62,18 @@ context.OperationId is not null &&

using (diagnostic.AnalyzeOperationComplexity(context))
{
var cacheId = context.CreateCacheId(context.OperationId);
var document = context.Document;
var operationDefinition =
context.Operation?.Definition ??
document.GetOperation(context.Request.OperationName);

if (!_cache.TryGetAnalyzer(cacheId, out var analyzer))
if (!_cache.TryGetAnalyzer(context.OperationId, out var analyzer))
{
analyzer = CompileAnalyzer(context, document, operationDefinition);
analyzer = CompileAnalyzer(
context,
document,
operationDefinition,
context.OperationId);
diagnostic.OperationComplexityAnalyzerCompiled(context);
}

Expand Down Expand Up @@ -101,34 +107,18 @@ context.OperationId is not null &&
private ComplexityAnalyzerDelegate CompileAnalyzer(
IRequestContext requestContext,
DocumentNode document,
OperationDefinitionNode operationDefinition)
OperationDefinitionNode operationDefinition,
string operationId)
{
var validatorContext = _contextPool.Get();
ComplexityAnalyzerDelegate? operationAnalyzer = null;

try
{
PrepareContext(requestContext, document, validatorContext);

_compiler.Visit(document, validatorContext);
var analyzers = (List<OperationComplexityAnalyzer>)validatorContext.List.Peek()!;

foreach (var analyzer in analyzers)
{
if (analyzer.OperationDefinitionNode == operationDefinition)
{
operationAnalyzer = analyzer.Analyzer;
}

_cache.TryAddAnalyzer(
requestContext.CreateCacheId(
CreateOperationId(
requestContext.DocumentId!,
analyzer.OperationDefinitionNode.Name?.Value)),
analyzer.Analyzer);
}

return operationAnalyzer!;
_compiler.Visit(operationDefinition, validatorContext);
var analyzer = (OperationComplexityAnalyzer)validatorContext.List.Pop()!;
_cache.TryAddAnalyzer(operationId, analyzer.Analyzer);
return analyzer.Analyzer;
}
finally
{
Expand Down
62 changes: 36 additions & 26 deletions src/HotChocolate/Core/src/Execution/Pipeline/PipelineTools.cs
Original file line number Diff line number Diff line change
@@ -1,47 +1,57 @@
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using HotChocolate.Execution.Processing;
using HotChocolate.Language;

namespace HotChocolate.Execution.Pipeline;

internal static class PipelineTools
{
private static readonly IReadOnlyDictionary<string, object?> _empty =
new Dictionary<string, object?>();
private static readonly VariableValueCollection _noVariables =
VariableValueCollection.Empty;
private static readonly Dictionary<string, object?> _empty = new();
private static readonly VariableValueCollection _noVariables = VariableValueCollection.Empty;

public static string CreateOperationId(string documentId, string? operationName) =>
operationName is null ? documentId : $"{documentId}+{operationName}";
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static string CreateOperationId(string documentId, string? operationName)
=> operationName is null ? documentId : $"{documentId}+{operationName}";

public static string CreateCacheId(this IRequestContext context, string operationId) =>
$"{context.Schema.Name}-{context.ExecutorVersion}-{operationId}";
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static string CreateCacheId(this IRequestContext context, string operationId)
=> $"{context.Schema.Name}-{context.ExecutorVersion}-{operationId}";

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static string CreateCacheId(
this IRequestContext context,
string documentId,
string? operationName)
=> CreateCacheId(context, CreateOperationId(documentId, operationName));

public static void CoerceVariables(
IRequestContext context,
VariableCoercionHelper coercionHelper,
IReadOnlyList<VariableDefinitionNode> variableDefinitions)
{
if (context.Variables is null)
if (context.Variables is not null)
{
if (variableDefinitions.Count == 0)
{
context.Variables = _noVariables;
}
else
return;
}

if (variableDefinitions.Count == 0)
{
context.Variables = _noVariables;
}
else
{
using (context.DiagnosticEvents.CoerceVariables(context))
{
using (context.DiagnosticEvents.CoerceVariables(context))
{
var coercedValues = new Dictionary<string, VariableValueOrLiteral>();

coercionHelper.CoerceVariableValues(
context.Schema,
variableDefinitions,
context.Request.VariableValues ?? _empty,
coercedValues);

context.Variables = new VariableValueCollection(coercedValues);
}
var coercedValues = new Dictionary<string, VariableValueOrLiteral>();

coercionHelper.CoerceVariableValues(
context.Schema,
variableDefinitions,
context.Request.VariableValues ?? _empty,
coercedValues);

context.Variables = new VariableValueCollection(coercedValues);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,18 @@ public static OperationDefinitionNode GetOperation(

for (var i = 0; i < length; i++)
{
if (definitions[i] is OperationDefinitionNode op)
if (definitions[i] is not OperationDefinitionNode op)
{
if (operation is null)
{
operation = op;
}
else
{
throw OperationResolverHelper_MultipleOperation(operation, op);
}
continue;
}

if (operation is null)
{
operation = op;
}
else
{
throw OperationResolverHelper_MultipleOperation(operation, op);
}
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/HotChocolate/Core/src/Execution/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -330,4 +330,7 @@
<data name="SelectionSetOptimizerContext_AddSelection_ResponseNameNotTheSame" xml:space="preserve">
<value>The provided response name must be the same as on the selection.</value>
</data>
<data name="ComplexityAnalyzerCompiler_Enter_OnlyOperations" xml:space="preserve">
<value>We only compile operations.</value>
</data>
</root>
Loading

0 comments on commit dc10c4b

Please sign in to comment.