Reworked Cache Handling in Pipeline (#6100)

ChilliCream · May 1, 2023 · b5c0e87 · b5c0e87
1 parent 86ff998
commit b5c0e87
Show file tree

Hide file tree

Showing 10 changed files with 496 additions and 211 deletions.
diff --git a/...xity/ComplexityAnalyzerCompilerVisitor.cs → .../Complexity/ComplexityAnalyzerCompiler.cs b/...xity/ComplexityAnalyzerCompilerVisitor.cs → .../Complexity/ComplexityAnalyzerCompiler.cs
@@ -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")!;
@@ -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,
@@ -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);
     }

diff --git a/src/HotChocolate/Core/src/Execution/Pipeline/OperationCacheMiddleware.cs b/src/HotChocolate/Core/src/Execution/Pipeline/OperationCacheMiddleware.cs
@@ -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;
@@ -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);
             }
         }

diff --git a/src/HotChocolate/Core/src/Execution/Pipeline/OperationComplexityMiddleware.cs b/src/HotChocolate/Core/src/Execution/Pipeline/OperationComplexityMiddleware.cs
@@ -1,5 +1,4 @@
 using System;
-using System.Collections.Generic;
 using System.Threading.Tasks;
 using HotChocolate.Execution.Caching;
 using HotChocolate.Execution.Options;
@@ -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(
@@ -40,7 +39,7 @@ public OperationComplexityMiddleware(
         _coercionHelper = coercionHelper ??
             throw new ArgumentNullException(nameof(coercionHelper));
 
-        _compiler = new ComplexityAnalyzerCompilerVisitor(_settings);
+        _compiler = new ComplexityAnalyzerCompiler(_settings);
     }
 
     public async ValueTask InvokeAsync(IRequestContext context)
@@ -59,15 +58,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, cacheId);
+                        analyzer = CompileAnalyzer(
+                            context,
+                            document,
+                            operationDefinition,
+                            context.OperationId);
                         diagnostic.OperationComplexityAnalyzerCompiled(context);
                     }
 
@@ -102,31 +104,17 @@ private ComplexityAnalyzerDelegate CompileAnalyzer(
         IRequestContext requestContext,
         DocumentNode document,
         OperationDefinitionNode operationDefinition,
-        string cacheId)
+        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.Equals(operationDefinition, SyntaxComparison.Syntax))
-                {
-                    operationAnalyzer = analyzer.Analyzer;
-
-                    _cache.TryAddAnalyzer(
-                        cacheId,
-                        analyzer.Analyzer);
-                }
-            }
-
-            return operationAnalyzer!;
+            _compiler.Visit(operationDefinition, validatorContext);
+            var analyzer = (OperationComplexityAnalyzer)validatorContext.List.Pop()!;
+            _cache.TryAddAnalyzer(operationId, analyzer.Analyzer);
+            return analyzer.Analyzer;
         }
         finally
         {

diff --git a/src/HotChocolate/Core/src/Execution/Pipeline/PipelineTools.cs b/src/HotChocolate/Core/src/Execution/Pipeline/PipelineTools.cs
@@ -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);
             }
         }
     }

diff --git a/src/HotChocolate/Core/src/Execution/Processing/OperationResolverHelper.cs b/src/HotChocolate/Core/src/Execution/Processing/OperationResolverHelper.cs
@@ -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);
                 }
             }
 

diff --git a/src/HotChocolate/Core/src/Execution/Properties/Resources.Designer.cs b/src/HotChocolate/Core/src/Execution/Properties/Resources.Designer.cs
diff --git a/src/HotChocolate/Core/src/Execution/Properties/Resources.resx b/src/HotChocolate/Core/src/Execution/Properties/Resources.resx
@@ -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>