Avoid per-element patch-path allocations in C# collection serializers

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs

@@ -25,14 +25,16 @@ private MethodBodyStatement CreateDictionarySerializationWithPatch(
             SerializationFormat serializationFormat,
             ScopedApi<JsonPatch> patchSnippet,
             string serializedName,
-            List<ValueExpression>? parentIndices = null)
+            List<ValueExpression>? parentIndices = null,
+            ValueExpression? parentHasPatch = null)
         {
             parentIndices ??= [];
 
             var jsonPathTemplate = BuildJsonPathForElement(serializedName, parentIndices);
+            var csharpJsonPathTemplate = BuildJsonPathForElement(serializedName, parentIndices, escapeForCSharpString: true);
             ValueExpression jsonPath = parentIndices.Count > 0
-                ? Utf8Snippets.GetBytes(new FormattableStringExpression(jsonPathTemplate, [.. parentIndices]).As<string>())
-                : LiteralU8($"$.{serializedName}");
+                ? Utf8Snippets.GetBytes(new FormattableStringExpression(csharpJsonPathTemplate, [.. parentIndices]).As<string>())
+                : LiteralU8(jsonPathTemplate);
 
             var foreachStatement = new ForEachStatement("item", dictionary, out KeyValuePairExpression keyValuePair);
 
@@ -73,7 +75,8 @@ private MethodBodyStatement CreateDictionarySerializationWithPatch(
                     patchSnippet,
                     serializationFormat,
                     serializedName,
-                    childIndices)
+                    childIndices,
+                    parentHasPatch)
             };
 
             var innerIfElseProcessorStatement = new IfElsePreprocessorStatement(
@@ -104,16 +107,36 @@ private MethodBodyStatement CreateListSerializationWithPatch(
             ScopedApi<JsonPatch> patchSnippet,
             SerializationFormat serializationFormat,
             string serializedName,
-            List<ValueExpression>? parentIndices = null)
+            List<ValueExpression>? parentIndices = null,
+            ValueExpression? parentHasPatch = null)
         {
             parentIndices ??= [];
             var indexDeclaration = Declare<int>("i", out var indexVar);
             var allIndices = new List<ValueExpression>(parentIndices) { indexVar };
             var jsonPathTemplate = BuildJsonPathForElement(serializedName, parentIndices);
-            var patchIsRemovedCondition = patchSnippet.IsRemoved(
+            var csharpJsonPathTemplate = BuildJsonPathForElement(serializedName, parentIndices, escapeForCSharpString: true);
+            // The prefix overload includes indexed descendants, unlike an exact-path Contains check.
+            // Nested collections under the same serialized property can reuse the parent guard.
+            MethodBodyStatement? hasPatchDeclaration = null;
+            ValueExpression hasPatch;
+            if (parentHasPatch == null)
+            {
+                hasPatchDeclaration = Declare(
+                    "hasPatch",
+                    typeof(bool),
+                    patchSnippet.Contains(LiteralU8("$"), LiteralU8(serializedName)),
+                    out var localHasPatch);
+                hasPatch = localHasPatch;
+            }
+            else
+            {
+                hasPatch = parentHasPatch;
+            }
+
+            var patchIsRemovedCondition = hasPatch.As<bool>().And(patchSnippet.IsRemoved(
                 Utf8Snippets.GetBytes(
-                    new FormattableStringExpression(jsonPathTemplate + $"[{{{parentIndices.Count}}}]", allIndices)
-                .As<string>()));
+                    new FormattableStringExpression(csharpJsonPathTemplate + $"[{{{parentIndices.Count}}}]", allIndices)
+                .As<string>())));
 
             // Handle model types with their own patch property
             if (ScmCodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(type, out var provider) &&
@@ -147,21 +170,28 @@ private MethodBodyStatement CreateListSerializationWithPatch(
                         patchSnippet,
                         serializationFormat,
                         serializedName,
-                        allIndices)
+                        allIndices,
+                        hasPatch)
                 }
             };
 
             var writeToPatchStatement = parentIndices.Count == 0
                 ? patchSnippet.WriteTo(_utf8JsonWriterSnippet, LiteralU8(jsonPathTemplate)).Terminate()
-                : patchSnippet.WriteTo(_utf8JsonWriterSnippet, Utf8Snippets.GetBytes(new FormattableStringExpression(jsonPathTemplate, parentIndices).As<string>())).Terminate();
+                : patchSnippet.WriteTo(_utf8JsonWriterSnippet, Utf8Snippets.GetBytes(new FormattableStringExpression(csharpJsonPathTemplate, parentIndices).As<string>())).Terminate();
 
-            return new[]
+            var listStatements = new List<MethodBodyStatement>
             {
-                _utf8JsonWriterSnippet.WriteStartArray(),
-                forStatement,
-                writeToPatchStatement,
-                _utf8JsonWriterSnippet.WriteEndArray()
+                _utf8JsonWriterSnippet.WriteStartArray()
             };
+            if (hasPatchDeclaration != null)
+            {
+                listStatements.Add(hasPatchDeclaration);
+            }
+
+            listStatements.Add(forStatement);
+            listStatements.Add(writeToPatchStatement);
+            listStatements.Add(_utf8JsonWriterSnippet.WriteEndArray());
+            return listStatements.ToArray();
         }
 
         private MethodBodyStatement CreateElementSerializationWithPatch(
@@ -170,7 +200,8 @@ private MethodBodyStatement CreateElementSerializationWithPatch(
             ScopedApi<JsonPatch> patchSnippet,
             SerializationFormat serializationFormat,
             string serializedName,
-            List<ValueExpression> currentIndices)
+            List<ValueExpression> currentIndices,
+            ValueExpression? parentHasPatch = null)
         {
             var nestedSerialization = elementType switch
             {
@@ -181,13 +212,15 @@ private MethodBodyStatement CreateElementSerializationWithPatch(
                     patchSnippet,
                     serializationFormat,
                     serializedName,
-                    currentIndices),
+                    currentIndices,
+                    parentHasPatch),
                 { IsDictionary: true } => CreateDictionarySerializationWithPatch(
                     new DictionaryExpression(elementType, element),
                     serializationFormat,
                     patchSnippet,
                     serializedName,
-                    currentIndices),
+                    currentIndices,
+                    parentHasPatch),
                 _ => null
             };
 
@@ -215,7 +248,7 @@ private IfElseStatement CreateConditionalPatchSerializationStatement(
             MethodBodyStatement writePropertySerializationStatement,
             MethodBodyStatement? elseStatementBody)
         {
-            string jsonPath = $"$.{serializedName}";
+            string jsonPath = BuildJsonPathForElement(serializedName, []);
             var ifPatchIsNotRemoved = new IfStatement(Not(_jsonPatchProperty!.As<JsonPatch>().IsRemoved(LiteralU8(jsonPath))))
             {
                 _utf8JsonWriterSnippet.WritePropertyName(serializedName),
@@ -605,10 +638,16 @@ private MethodProvider BuildActiveItemsMethod(PropertyProvider property)
             {
                 isActive = item.Equal(Null).Or(isActive);
             }
+            var serializedName = GetJsonSerializedName(property.WireInfo!);
+            var hasPatchDeclaration = Declare(
+                "hasPatch",
+                typeof(bool),
+                _jsonPatchProperty!.As<JsonPatch>().Contains(LiteralU8("$"), LiteralU8(serializedName)),
+                out var hasPatch);
             var itemPath = Utf8Snippets.GetBytes(new FormattableStringExpression(
-                BuildJsonPathForElement(GetJsonSerializedName(property.WireInfo!), [indexVar]),
+                BuildJsonPathForElement(serializedName, [indexVar], escapeForCSharpString: true),
                 [indexVar]).As<string>());
-            isActive = Not(_jsonPatchProperty!.As<JsonPatch>().IsRemoved(itemPath)).And(isActive);
+            isActive = Not(hasPatch).Or(Not(_jsonPatchProperty!.As<JsonPatch>().IsRemoved(itemPath))).And(isActive);
             var forStatement = new ForStatement(
                 indexDeclaration.Assign(Literal(0)),
                 indexVar.LessThan(((ValueExpression)property).Property(lengthPropertyName)),
@@ -626,6 +665,7 @@ private MethodProvider BuildActiveItemsMethod(PropertyProvider property)
                 {
                     YieldBreak()
                 },
+                hasPatchDeclaration,
                 forStatement
             };
 
@@ -656,15 +696,10 @@ private List<PropertyProvider> GetQualifyingDynamicListProperties()
 
 #pragma warning restore SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
 
-        private static string BuildJsonPathForElement(string propertySerializedName, List<ValueExpression> indices)
+        private static string BuildJsonPathForElement(string propertySerializedName, List<ValueExpression> indices, bool escapeForCSharpString = false)
         {
             var count = indices.Count;
-            if (count == 0)
-            {
-                return $"$.{propertySerializedName}";
-            }
-
-            var result = $"$.{propertySerializedName}";
+            var result = BuildJsonPathForProperty(propertySerializedName, escapeForCSharpString);
             for (int i = 0; i < count; i++)
             {
                 result += indices[i] is MemberExpression
@@ -675,6 +710,17 @@ private static string BuildJsonPathForElement(string propertySerializedName, Lis
             return result;
         }
 
+        private static string BuildJsonPathForProperty(string propertySerializedName, bool escapeForCSharpString)
+        {
+            var jsonPath = propertySerializedName.Contains('.')
+                ? $"$[\"{propertySerializedName}\"]"
+                : $"$.{propertySerializedName}";
+
+            return escapeForCSharpString
+                ? jsonPath.Replace("\"", "\\\"")
+                : jsonPath;
+        }
+
         private static ValueExpression GetDeserializationMethodInvocationForType(
             ModelProvider model,
             ScopedApi element,

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/ModelReaderWriterValidation/TestProjects/Sample_TypeSpec/DynamicModelTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Buffers;
 using System.ClientModel.Primitives;
 using System.Collections.Generic;
 using System.Linq;
@@ -369,6 +370,65 @@ public void JsonPatchRemove_NullDynamicListElementSnapshot(bool onlyNull)
             Assert.That(Encoding.UTF8.GetString(json), Is.EqualTo(onlyNull ? "[]" : """[{"bar":"present"},null]"""));
         }
 
+        [TestCase(false)]
+        [TestCase(true)]
+        public void JsonModelWrite_UnpatchedCollectionDoesNotAllocatePerElement(bool unrelatedPatch)
+        {
+            var model = new NullableDynamicModel
+            {
+                Children = new AnotherDynamicModel[256]
+            };
+#pragma warning disable SCME0001
+            if (unrelatedPatch)
+            {
+                model.Patch.Set("$.unrelated"u8, 1);
+            }
+#pragma warning restore SCME0001
+
+            var buffer = new ArrayBufferWriter<byte>();
+            using var writer = new Utf8JsonWriter(buffer);
+            var jsonModel = (IJsonModel<NullableDynamicModel>)model;
+            jsonModel.Write(writer, ModelReaderWriterOptions.Json);
+            writer.Flush();
+            buffer.Clear();
+            writer.Reset(buffer);
+
+            long before = GC.GetAllocatedBytesForCurrentThread();
+            jsonModel.Write(writer, ModelReaderWriterOptions.Json);
+            writer.Flush();
+            long allocated = GC.GetAllocatedBytesForCurrentThread() - before;
+
+            Assert.That(allocated, Is.LessThan(1024), "Indexed patch paths must not allocate for each unpatched element.");
+            using var document = JsonDocument.Parse(buffer.WrittenMemory);
+            Assert.That(document.RootElement.GetProperty("children").GetArrayLength(), Is.EqualTo(256));
+        }
+
+        [TestCase(false)]
+        [TestCase(true)]
+        public void JsonPatchRemove_ChildRootWithUnpatchedParentCollection(bool unrelatedPatch)
+        {
+            var removed = new AnotherDynamicModel("removed");
+            var model = new NullableDynamicModel
+            {
+                Children = [null, removed, new AnotherDynamicModel("present")]
+            };
+
+#pragma warning disable SCME0001
+            removed.Patch.Remove("$"u8);
+            if (unrelatedPatch)
+            {
+                model.Patch.Set("$.unrelated"u8, 1);
+            }
+            Assert.That(model.Patch.Contains("$"u8, "children"u8), Is.False);
+            var snapshot = model.Patch.GetJson("$.children"u8);
+#pragma warning restore SCME0001
+
+            Assert.That(Encoding.UTF8.GetString(snapshot), Is.EqualTo("""[null,{"bar":"present"}]"""));
+            var data = ModelReaderWriter.Write(model, ModelReaderWriterOptions.Json, SampleTypeSpecContext.Default);
+            using var document = JsonDocument.Parse(data);
+            Assert.That(document.RootElement.GetProperty("children").GetRawText(), Is.EqualTo("""[null,{"bar":"present"}]"""));
+        }
+
         private static NullableDynamicModel CreateModelWithRemovedDynamicListElements(string propertyName, bool onlyNull)
         {
             var items = onlyNull ? "[null]" : """[null,{"bar":"present"},null]""";

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/DynamicModelSerializationTests.cs

@@ -433,6 +433,38 @@ public void PropagateModelListPropertyHelperMethods()
             Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
         }
 
+        [Test]
+        public void DottedSerializedNameCollectionPatchGuards()
+        {
+            var inputModel = InputFactory.Model(
+                "dynamicModel",
+                isDynamicModel: true,
+                properties:
+                [
+                    InputFactory.Property(
+                        "children",
+                        InputFactory.Array(InputFactory.Model(
+                            "anotherDynamic",
+                            isDynamicModel: true,
+                            properties:
+                            [
+                                InputFactory.Property("value", InputPrimitiveType.String, isRequired: true)
+                            ])),
+                        serializedName: "foo.bar")
+                ]);
+
+            MockHelpers.LoadMockGenerator(inputModels: () => [inputModel]);
+            var model = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel) as ClientModel.Providers.ScmModelProvider;
+
+            Assert.IsNotNull(model);
+            var serialization = model!.SerializationProviders.Single();
+            var writer = new TypeProviderWriter(new FilteredMethodsTypeProvider(
+                serialization,
+                name => name is "JsonModelWriteCore" or "ActiveChildren"));
+
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), writer.Write().Content);
+        }
+
         [Test]
         public void PropagateModelDictionaryProperty()
         {

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/DottedSerializedNameCollectionPatchGuards.cs

@@ -0,0 +1,71 @@
+// <auto-generated/>
+
+#nullable disable
+
+using System;
+using System.ClientModel.Primitives;
+using System.Collections.Generic;
+using System.Text;
+using System.Text.Json;
+using Sample.Models;
+
+namespace Sample
+{
+    public partial class DynamicModel
+    {
+        protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
+        {
+            string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.DynamicModel>)this).GetFormatFromOptions(options) : options.Format;
+            if ((format != "J"))
+            {
+                throw new global::System.FormatException($"The model {nameof(global::Sample.Models.DynamicModel)} does not support writing '{format}' format.");
+            }
+#pragma warning disable SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+            if (Patch.Contains("$[\"foo.bar\"]"u8))
+            {
+                if (!Patch.IsRemoved("$[\"foo.bar\"]"u8))
+                {
+                    writer.WritePropertyName("foo.bar"u8);
+                    Patch.WriteTo(writer, "$[\"foo.bar\"]"u8);
+                }
+            }
+            else if (global::Sample.Optional.IsCollectionDefined(Children))
+            {
+                writer.WritePropertyName("foo.bar"u8);
+                writer.WriteStartArray();
+                bool hasPatch = Patch.Contains("$"u8, "foo.bar"u8);
+                for (int i = 0; (i < Children.Count); i++)
+                {
+                    if (((hasPatch && Patch.IsRemoved(global::System.Text.Encoding.UTF8.GetBytes($"$[\"foo.bar\"][{i}]"))) || ((Children[i] != null) && Children[i].Patch.IsRemoved("$"u8))))
+                    {
+                        continue;
+                    }
+                    writer.WriteObjectValue<global::Sample.Models.AnotherDynamic>(Children[i], options);
+                }
+                Patch.WriteTo(writer, "$[\"foo.bar\"]"u8);
+                writer.WriteEndArray();
+            }
+
+            Patch.WriteTo(writer);
+#pragma warning restore SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+        }
+
+#pragma warning disable SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+        private global::System.Collections.Generic.IEnumerable<global::Sample.Models.AnotherDynamic> ActiveChildren()
+        {
+            if (!global::Sample.Optional.IsCollectionDefined(Children))
+            {
+                yield break;
+            }
+            bool hasPatch = Patch.Contains("$"u8, "foo.bar"u8);
+            for (int i = 0; (i < Children.Count); i++)
+            {
+                if (((!hasPatch || !Patch.IsRemoved(global::System.Text.Encoding.UTF8.GetBytes($"$[\"foo.bar\"][{i}]"))) && ((Children[i] == null) || !Children[i].Patch.IsRemoved("$"u8))))
+                {
+                    yield return Children[i];
+                }
+            }
+        }
+#pragma warning restore SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+    }
+}