[DO NOT MERGE] Explore consistency and performance frameworks for writing responses

Directory.Packages.props

@@ -13,6 +13,7 @@
     <PackageVersion Include="CommandLineParser" Version="2.9.1" />
     <PackageVersion Include="JsonPath.Net" Version="1.1.6" />
     <PackageVersion Include="KeraLua" Version="1.4.9" />
+    <PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="5.3.0" />
     <PackageVersion Include="Microsoft.Identity.Client" Version="4.76.0" />
     <PackageVersion Include="NUnit" Version="4.5.1" />
     <PackageVersion Include="NUnit3TestAdapter" Version="6.2.0" />

Garnet.slnx

@@ -1,4 +1,5 @@
 <Solution>
+  <Folder Name="/.azure/" />
   <Folder Name="/.azure/pipelines/">
     <File Path=".azure/pipelines/azure-pipelines-compliance-policheck.yml" />
     <File Path=".azure/pipelines/azure-pipelines-compliance.yml" />
@@ -12,11 +13,15 @@
     <File Path=".azure/pipelines/credscan-exclusion.json" />
     <File Path=".azure/pipelines/policheck-exclusion.xml" />
   </Folder>
+  <Folder Name="/analyzer/">
+    <Project Path="analyzer/Garnet.analyzers/Garnet.analyzers.csproj" Id="903d62cb-f5ad-4a08-b11c-99ecda74317f" />
+  </Folder>
   <Folder Name="/benchmark/">
     <Project Path="benchmark/BDN.benchmark/BDN.benchmark.csproj" />
     <Project Path="benchmark/Device.benchmark/Device.benchmark.csproj" />
     <Project Path="benchmark/Resp.benchmark/Resp.benchmark.csproj" />
   </Folder>
+  <Folder Name="/hosting/" />
   <Folder Name="/hosting/Windows/">
     <Project Path="hosting/Windows/Garnet.worker/Garnet.worker.csproj" />
   </Folder>
@@ -29,12 +34,8 @@
     <Project Path="libs/server/Garnet.server.csproj" />
   </Folder>
   <Folder Name="/libs/Tsavorite/">
-    <Project Path="libs/storage/Tsavorite/cs/src/core/Tsavorite.core.csproj">
-      <Platform Solution="*|x64" Project="x64" />
-    </Project>
-    <Project Path="libs/storage/Tsavorite/cs/src/devices/AzureStorageDevice/Tsavorite.devices.AzureStorageDevice.csproj">
-      <Platform Solution="*|x64" Project="x64" />
-    </Project>
+    <Project Path="libs/storage/Tsavorite/cs/src/core/Tsavorite.core.csproj" />
+    <Project Path="libs/storage/Tsavorite/cs/src/devices/AzureStorageDevice/Tsavorite.devices.AzureStorageDevice.csproj" />
   </Folder>
   <Folder Name="/main/">
     <Project Path="main/GarnetServer/GarnetServer.csproj" />

analyzer/Garnet.analyzers/AvoidRespWriteUtilsAnalyzer.cs

@@ -0,0 +1,151 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.IO;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace Garnet.analyzers
+{
+    /// <summary>
+    /// Analyzer which flags uses of RespWriteUtils outside of the RespServerSessionOutput helper methods.
+    /// 
+    /// The intent is to force consistency in output for handling large outputs, error tracking, constant use, etc.
+    /// </summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class AvoidRespWriteUtilsAnalyzer : DiagnosticAnalyzer
+    {
+        private static DiagnosticDescriptor AvoidRespWriteUtils { get; } = new("GARNET0001", "Avoid direct use of RespWriteUtils", "If possible use RespServerSession.{0} instead of RespWriteUtils.{1}", "Correctness", DiagnosticSeverity.Warning, isEnabledByDefault: true);
+
+        /// <inheritdoc/>
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = [AvoidRespWriteUtils];
+
+        /// <inheritdoc/>
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.ReportDiagnostics | GeneratedCodeAnalysisFlags.Analyze);
+            context.EnableConcurrentExecution();
+
+            context.RegisterCompilationStartAction(
+                static compilationStartContext =>
+                {
+                    var respWriteUtilsType = compilationStartContext.Compilation.GetTypeByMetadataName("Garnet.common.RespWriteUtils");
+                    var respServerSessionType = compilationStartContext.Compilation.GetTypeByMetadataName("Garnet.server.RespServerSession");
+
+                    if (respWriteUtilsType is not null && respServerSessionType is not null)
+                    {
+                        var suggestionLookup = BuildLookup(respWriteUtilsType, respServerSessionType);
+
+                        compilationStartContext.RegisterSyntaxNodeAction(syntaxNodeContext => AnalyzeCaller(syntaxNodeContext, respWriteUtilsType, suggestionLookup), SyntaxKind.InvocationExpression);
+                    }
+                }
+            );
+
+            // Build a map of TryWriteXXX methods on RespWriteUtils -> WriteXXX methods on RespServerSession
+            static Dictionary<string, string> BuildLookup(INamedTypeSymbol respWriteUtilsType, INamedTypeSymbol respServerSessionType)
+            {
+                var tryWriteMethods = new HashSet<string>();
+
+                foreach (var member in respWriteUtilsType.GetMembers())
+                {
+                    if (member is not IMethodSymbol mtdSymbol)
+                    {
+                        continue;
+                    }
+
+                    if (!member.Name.StartsWith("TryWrite"))
+                    {
+                        continue;
+                    }
+
+                    _ = tryWriteMethods.Add(member.Name);
+                }
+
+                var lookup = new Dictionary<string, string>();
+                foreach (var member in respServerSessionType.GetMembers())
+                {
+                    if (member is not IMethodSymbol mtdSymbol)
+                    {
+                        continue;
+                    }
+
+                    // Check that method is declared in RespServerSessionOutput.cs, otherwise we don't consider it a candidate for flagging
+                    if (!member.DeclaringSyntaxReferences.Any(static decl => Path.GetFileName(decl.SyntaxTree.FilePath) == "RespServerSessionOutput.cs"))
+                    {
+                        continue;
+                    }
+
+                    if (!member.Name.StartsWith("Write"))
+                    {
+                        continue;
+                    }
+
+                    var tryEquivalent = $"Try{member.Name}";
+                    if (!tryWriteMethods.Contains(tryEquivalent))
+                    {
+                        continue;
+                    }
+
+                    lookup[tryEquivalent] = member.Name;
+                }
+
+                return lookup;
+            }
+        }
+
+        /// <summary>
+        /// Flag all calls to methods looking like RespWriteUtils.TryXXX
+        /// </summary>
+        private static void AnalyzeCaller(SyntaxNodeAnalysisContext context, INamedTypeSymbol respWriteUtilsType, Dictionary<string, string> candidateLookup)
+        {
+            if (context.Node is not InvocationExpressionSyntax invoke)
+            {
+                return;
+            }
+
+            if (invoke.Expression is not MemberAccessExpressionSyntax memberAccess)
+            {
+                return;
+            }
+
+            if (memberAccess.Name is not IdentifierNameSyntax methodName)
+            {
+                return;
+            }
+
+            // Quickly filter out anything that doesn't look like a TryXXX method
+            if (string.IsNullOrEmpty(methodName.Identifier.Text) || !methodName.Identifier.Text.StartsWith("Try"))
+            {
+                return;
+            }
+
+            // Filter out anything in RespServerSessionOutput.cs, as it's intended to use these methods
+            if (Path.GetFileName(context.Node.SyntaxTree.FilePath) == "RespServerSessionOutput.cs")
+            {
+                return;
+            }
+
+            // Ignore anything that isn't a call to RespWriteUtils
+            var leftHandType = context.SemanticModel.GetSymbolInfo(memberAccess.Expression);
+            if (leftHandType.Symbol is null || !SymbolEqualityComparer.Default.Equals(leftHandType.Symbol, respWriteUtilsType))
+            {
+                return;
+            }
+
+            // Lookup equivalent method, and raise diagnostic
+            if (!candidateLookup.TryGetValue(methodName.Identifier.Text, out var rewriteTo))
+            {
+                return;
+            }
+
+            // Raise the actual diagnostic
+            var diag = Diagnostic.Create(AvoidRespWriteUtils, context.Node.GetLocation(), rewriteTo, methodName.Identifier.Text);
+            context.ReportDiagnostic(diag);
+        }
+    }
+}

analyzer/Garnet.analyzers/CollapseRepeatedConstantWriteCallsAnalyzer.cs

@@ -0,0 +1,184 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.IO;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Text;
+
+namespace Garnet.analyzers
+{
+    /// <summary>
+    /// Flag cases where two or more sequential calls to WriteXXX helpers (defined in RespServerSession) where the arguments are constants.
+    /// 
+    /// IE.
+    ///   WriteArrayLength(2)
+    ///   WriteBulkString(CmdStrings.Foo)
+    /// </summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class CollapseRepeatedConstantWriteCallsAnalyzer : DiagnosticAnalyzer
+    {
+        private static DiagnosticDescriptor CollapseRepeatedConstantWriteCalls { get; } = new("GARNET0007", "Collapse Repeated Constant Write Calls", "Define a CmdString constant for these {0} consecutive WriteXXX helper calls", "Correctness", DiagnosticSeverity.Warning, isEnabledByDefault: true);
+
+        /// <inheritdoc/>
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = [CollapseRepeatedConstantWriteCalls];
+
+        /// <inheritdoc/>
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.ReportDiagnostics | GeneratedCodeAnalysisFlags.Analyze);
+            context.EnableConcurrentExecution();
+
+            context.RegisterCompilationStartAction(
+                compilationStartContext =>
+                {
+                    var respServerSessionType = compilationStartContext.Compilation.GetTypeByMetadataName("Garnet.server.RespServerSession");
+                    var cmdStringsType = compilationStartContext.Compilation.GetTypeByMetadataName("Garnet.server.CmdStrings");
+
+                    if (respServerSessionType is not null && cmdStringsType is not null)
+                    {
+                        var lookup = BuildLookup(respServerSessionType);
+
+                        compilationStartContext.RegisterSyntaxNodeAction(
+                            syntaxNodeContext => AnalyzeCodeBlockForRepeatedConstantWriteCalls(syntaxNodeContext, cmdStringsType, lookup),
+                            SyntaxKind.Block
+                        );
+                    }
+                }
+            );
+
+            // Lookup all WriteXXX methods declared in RespServerSessionOutput.cs
+            static HashSet<IMethodSymbol> BuildLookup(INamedTypeSymbol respServerSessionType)
+            {
+                var lookup = new HashSet<IMethodSymbol>(SymbolEqualityComparer.Default);
+                foreach (var member in respServerSessionType.GetMembers())
+                {
+                    if (member is not IMethodSymbol mtdSymbol)
+                    {
+                        continue;
+                    }
+
+                    // Check that method is declared in RespServerSessionOutput.cs, otherwise we don't consider it a candidate for flagging
+                    if (!member.DeclaringSyntaxReferences.Any(static decl => Path.GetFileName(decl.SyntaxTree.FilePath) == "RespServerSessionOutput.cs"))
+                    {
+                        continue;
+                    }
+
+                    if (!member.Name.StartsWith("Write"))
+                    {
+                        continue;
+                    }
+
+                    _ = lookup.Add(mtdSymbol);
+                }
+
+                return lookup;
+            }
+        }
+
+        /// <summary>
+        /// Raise the <see cref="CollapseRepeatedConstantWriteCalls"/> if 2 or more calls to methods in <paramref name="writeMethods"/> appear in a row with constant arguments.
+        /// </summary>
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("MicrosoftCodeAnalysisCorrectness", "RS1024:Symbols should be compared for equality", Justification = "writeMethods is constructed with the appropriate comparer")]
+        private static void AnalyzeCodeBlockForRepeatedConstantWriteCalls(SyntaxNodeAnalysisContext blockAnalyzeContext, INamedTypeSymbol cmdStringsType, HashSet<IMethodSymbol> writeMethods)
+        {
+            if (blockAnalyzeContext.Node is not BlockSyntax blockSyntax)
+            {
+                return;
+            }
+
+            if (blockSyntax.Statements.Count <= 1)
+            {
+                return;
+            }
+
+            // Walk through all statements and find runs of 2+ constant calls to methods in writeMethods
+            var constantWriteCalls = new List<StatementSyntax>();
+            foreach (var statement in blockSyntax.Statements)
+            {
+                // Skip trivia
+                if (statement.IsStructuredTrivia)
+                {
+                    continue;
+                }
+
+                // Only look at statements of the form:
+                //    WriteXXX();
+                if (statement is ExpressionStatementSyntax expressionSyntax && expressionSyntax.Expression is InvocationExpressionSyntax invocationSyntax && invocationSyntax.Expression is IdentifierNameSyntax mtdNameSyntax && mtdNameSyntax.Identifier.Text.StartsWith("Write"))
+                {
+                    var methodType = blockAnalyzeContext.SemanticModel.GetSymbolInfo(mtdNameSyntax, blockAnalyzeContext.CancellationToken);
+                    if (methodType.Symbol is not null && writeMethods.Contains(methodType.Symbol) && IsConstantMethodCall(blockAnalyzeContext, invocationSyntax, cmdStringsType))
+                    {
+                        // This is a constant, remember it and move on
+                        constantWriteCalls.Add(expressionSyntax);
+                        continue;
+                    }
+                }
+
+                // If we encountered something we _didn't_ remember then we need to raise diagnostics for the previously found statements
+                if (constantWriteCalls.Count > 1)
+                {
+                    RaiseDiagnostic(blockAnalyzeContext, constantWriteCalls);
+                }
+
+                // A single statement cannot be collapsed
+                if (constantWriteCalls.Count != 0)
+                {
+                    constantWriteCalls.Clear();
+                }
+            }
+
+            // At the end of the block, flush any previously found statements
+            if (constantWriteCalls.Count > 1)
+            {
+                RaiseDiagnostic(blockAnalyzeContext, constantWriteCalls);
+            }
+
+            // Determine if a call to the given WriteXXX method should be considered a constant call
+            static bool IsConstantMethodCall(SyntaxNodeAnalysisContext blockAnalyzeContext, InvocationExpressionSyntax invocationSyntax, INamedTypeSymbol cmdStringsType)
+            {
+                foreach (var arg in invocationSyntax.ArgumentList.Arguments)
+                {
+                    if (arg.Expression is MemberAccessExpressionSyntax memberAccess)
+                    {
+                        var beingAccessed = blockAnalyzeContext.SemanticModel.GetSymbolInfo(memberAccess.Expression, blockAnalyzeContext.CancellationToken);
+                        if (beingAccessed.Symbol is not null && SymbolEqualityComparer.Default.Equals(beingAccessed.Symbol, cmdStringsType))
+                        {
+                            continue;
+                        }
+                    }
+
+                    var constantValue = blockAnalyzeContext.SemanticModel.GetConstantValue(arg.Expression);
+                    if (constantValue.HasValue)
+                    {
+                        continue;
+                    }
+
+                    // Non-constant, can't collapse
+                    return false;
+                }
+
+                // All constants (including no argument), we can collapse
+                return true;
+            }
+
+            // Raise a diagnostic that covers all the consecutive statements passed
+            static void RaiseDiagnostic(SyntaxNodeAnalysisContext blockAnalyzeContext, List<StatementSyntax> toFlag)
+            {
+                var start = toFlag[0].GetLocation();
+                var end = toFlag[toFlag.Count - 1].GetLocation();
+
+                var wholeSpan = new TextSpan(start.SourceSpan.Start, end.SourceSpan.End - start.SourceSpan.Start);
+                var location = Location.Create(start.SourceTree, wholeSpan);
+
+                var diag = Diagnostic.Create(CollapseRepeatedConstantWriteCalls, location, toFlag.Count);
+                blockAnalyzeContext.ReportDiagnostic(diag);
+            }
+        }
+    }
+}

analyzer/Garnet.analyzers/Garnet.analyzers.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFrameworks>netstandard2.0</TargetFrameworks>
+	  <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
+		<NoWarn>$(NoWarn);RS2008</NoWarn>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" />
+  </ItemGroup>
+
+</Project>