From 4b553e21f3ff1881fa88a2595fbdec521f377820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Sat, 26 Sep 2026 20:20:28 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(serialization):=20read=20a=20registered?= =?UTF-8?q?=20$type-first=20object=20straight=20from=20the=20reader=20?= =?UTF-8?q?=E2=80=94=20no=20JsonDocument=20+=20string=20copy=20per=20nesti?= =?UTF-8?q?ng=20level?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The [HEAPSTEP] sampler (#5665), live on ci.9408, names the same allocation mix in every heap step on both portals: String 53-56%, Byte[] 25-31%, JsonDocument 2-4%. That is one JSON value held as UTF-8 and again as UTF-16. ObjectPolymorphicConverter.ReadObject did exactly that for every object-typed value: JsonDocument.ParseValue (a byte[] copy), GetRawText() (a UTF-16 string), then Deserialize(string). Every nested object member re-enters the converter, so it repeated once per nesting level. - $type first + registered type + no reference metadata: deserialize from the reader directly (probe on a reader copy; declining leaves the reader untouched). - every other shape: the general path, now deserializing from UTF-8 instead of a string (JsonElementNormalizer.Deserialize replaces GetNormalizedRawText, also for the read-only collection converters). PolymorphicReadAllocationTest: one read of a value nested 8 levels with a 400,000-byte leaf allocated 4,006,624 bytes before, 402,024 after. Refs #5555, #4824 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Data/Architecture/WhichKindOfSilence.md | 30 +++++ .../ObjectPolymorphicConverter.cs | 75 ++++++++++- .../ReadOnlyCollectionConverterFactory.cs | 47 +++---- .../PolymorphicReadAllocationTest.cs | 124 ++++++++++++++++++ 4 files changed, 252 insertions(+), 24 deletions(-) create mode 100644 test/MeshWeaver.Messaging.Hub.Test/PolymorphicReadAllocationTest.cs diff --git a/src/MeshWeaver.Documentation/Data/Architecture/WhichKindOfSilence.md b/src/MeshWeaver.Documentation/Data/Architecture/WhichKindOfSilence.md index b33dde3309..e05ec80c6e 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/WhichKindOfSilence.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/WhichKindOfSilence.md @@ -153,6 +153,36 @@ How to read it: against the fresh map instead of adding to one nobody reads again. - **Find it** with the same `Logs` action as the heartbeat: `query: "HEAPSTEP\\] tick="`, per pod. +### What the first readings named + +The first live readings came from image `3.0.0-ci.9408` on both portals, 2026-09-26 between 10:00Z +and 18:15Z. `Logs` actions `Ops/Actions/logs-memexcloud-20260926-heapstep-5555` and +`…/logs-memex-20260926-heapstep-5555` read 43 and 88 lines, none cut. + +- **Every step past warm-up has the same allocation mix.** `System.String` is 53–56 % of the sampled + bytes, `System.Byte[]` 25–31 %, and `System.Text.Json.JsonDocument` 2–4 %. The mix is the same on + memex and on memex-cloud, and across pods. A window allocated 4–7 GiB in 10 s. Only the first ticks + after boot differ, when assembly loading (`CoffHeader`) and serializer set-up are in the mix. +- **The steps are churn, not retention.** With server GC, a gen-0 budget is gigabytes, so + `GC.GetTotalMemory(false)` includes garbage until the next collection. On + `memex-portal-deployment-6c7669df84-nbrdx`, tick 314 read `heap=4.44GiB`. Tick 315 ran two gen-0, + one gen-1 and one gen-2 collection and read `3.03GiB`. At the 512 MiB threshold, a `[HEAPSTEP]` line + here marks an allocation burst. It does not by itself mark a leak. +- **String ≈ 2 × Byte[] plus a `JsonDocument` is one JSON value held twice**: once as UTF-8 and once + as UTF-16, which takes twice the bytes. `ObjectPolymorphicConverter.ReadObject` did exactly that for + every `object`-typed value. It copied the value into a `JsonDocument`, took `GetRawText()` as a + string, and deserialized that string again. Each nested `object` member re-enters the converter, so + this happened once per nesting level. `PolymorphicReadAllocationTest` measures one read of a value + nested 8 levels deep, with a 400,000-byte leaf string: **4,006,624 bytes** allocated before the fix, + **402,024** after. The leaf itself is the floor. +- **The fix.** When `$type` is the first property, names a registered type, and there is no + reference metadata, the converter now deserializes straight from the reader, with no document and + no string. Every other shape takes the general path as before, and that path now deserializes from + UTF-8 instead of a string. +- **Not yet established:** whether this converter is the dominant caller in production. The sampler + names types, not call sites. The falsifier is the `String` share on the first image that carries this + change. If it stays above 50 %, another caller holds the JSON as text. + `HeapStepNamesItsAllocatorTest` pins the rule with pure tests. It also has a live test: it allocates ~160 MiB of a marker type and requires the sampler to attribute at least 64 MiB of it to that type. That live test is the positive control. A sampler that received nothing would make every real line diff --git a/src/MeshWeaver.Messaging.Hub/Serialization/ObjectPolymorphicConverter.cs b/src/MeshWeaver.Messaging.Hub/Serialization/ObjectPolymorphicConverter.cs index 5a70cda40c..62b99309b3 100644 --- a/src/MeshWeaver.Messaging.Hub/Serialization/ObjectPolymorphicConverter.cs +++ b/src/MeshWeaver.Messaging.Hub/Serialization/ObjectPolymorphicConverter.cs @@ -115,6 +115,16 @@ public override bool CanConvert(Type typeToConvert) } private object ReadObject(ref Utf8JsonReader reader, JsonSerializerOptions options) { + // 🚨 The common shape — "$type" first, a registered type, no reference metadata — is read + // STRAIGHT from the reader (MeshWeaver#5555). The general path below materialises the object + // as a JsonDocument (a byte[] copy of the value plus its metadata database) and then as a + // UTF-16 string (2× the bytes) only to deserialize that string again — and it does so once + // PER NESTING LEVEL, because every `object`-typed member re-enters this method. That is the + // String ≈ 2 × Byte[] + JsonDocument signature the [HEAPSTEP] sampler found dominating every + // multi-hundred-MiB heap step on both portals. + if (TryReadRegisteredTypeFromReader(ref reader, options, out var direct)) + return direct; + using var doc = JsonDocument.ParseValue(ref reader); var root = doc.RootElement; @@ -131,8 +141,7 @@ private object ReadObject(ref Utf8JsonReader reader, JsonSerializerOptions optio { // Deserialize to the specific type using cleaned JSON // Normalize to ensure $type is first (required for parameterized constructor types) - var json = JsonElementNormalizer.GetNormalizedRawText(cleanedElement); - return JsonSerializer.Deserialize(json, typeInfo!.Type, options)!; + return JsonElementNormalizer.Deserialize(cleanedElement, typeInfo!.Type, options)!; } catch (Exception ex) when ( ex is JsonException @@ -182,6 +191,68 @@ or InvalidOperationException return cleanedElement.Clone(); } + private static readonly string[] MetadataPropertyNames = ["$id", "$ref", "$values", "$defs"]; + + /// + /// Deserializes an object whose FIRST property is a registered $type directly from the + /// reader — no , no string. Declines (returns false, reader untouched) + /// for every other shape, which the general path then handles exactly as before: $type + /// elsewhere or absent, an unregistered type (self-heal + warning), or reference metadata + /// ($id/$ref/…, which the general path strips). + /// + private bool TryReadRegisteredTypeFromReader( + ref Utf8JsonReader reader, JsonSerializerOptions options, out object result) + { + result = null!; + // Probe on a COPY: Utf8JsonReader is a struct, so declining leaves the caller's reader at the + // StartObject it was handed. A converter's reader holds the whole value (System.Text.Json + // reads ahead before invoking a custom converter), so TrySkip only fails on malformed input, + // and then the general path produces the same error it always did. + var probe = reader; + if (!probe.Read() || probe.TokenType != JsonTokenType.PropertyName + || !probe.ValueTextEquals(EntitySerializationExtensions.TypeProperty)) + return false; + if (!probe.Read() || probe.TokenType != JsonTokenType.String) + return false; + var typeName = probe.GetString(); + if (string.IsNullOrEmpty(typeName) || !typeRegistry.TryGetType(typeName, out var typeInfo)) + return false; + + while (probe.Read() && probe.TokenType == JsonTokenType.PropertyName) + { + foreach (var metadata in MetadataPropertyNames) + if (probe.ValueTextEquals(metadata)) + return false; + if (!probe.Read() || !probe.TrySkip()) + return false; + } + if (probe.TokenType != JsonTokenType.EndObject) + return false; + + var start = reader; + try + { + result = JsonSerializer.Deserialize(ref reader, typeInfo!.Type, options)!; + return true; + } + catch (Exception ex) when ( + ex is JsonException + or NotSupportedException + or InvalidOperationException + or ArgumentException) + { + // Same contract as the general path: a registered type whose stored JSON no longer fits + // is preserved as raw JSON (a throw faults the node read → wedged grain), logged loud. + logger?.LogWarning(ex, + "Content for '{TypeName}' could not be deserialized; preserving raw JSON", + typeName); + reader = start; + using var doc = JsonDocument.ParseValue(ref reader); + result = doc.RootElement.Clone(); + return true; + } + } + private static JsonElement StripMetadataProperties(JsonElement element) { if (element.ValueKind != JsonValueKind.Object) diff --git a/src/MeshWeaver.Messaging.Hub/Serialization/ReadOnlyCollectionConverterFactory.cs b/src/MeshWeaver.Messaging.Hub/Serialization/ReadOnlyCollectionConverterFactory.cs index eb18e27424..7e687a6748 100644 --- a/src/MeshWeaver.Messaging.Hub/Serialization/ReadOnlyCollectionConverterFactory.cs +++ b/src/MeshWeaver.Messaging.Hub/Serialization/ReadOnlyCollectionConverterFactory.cs @@ -6,31 +6,25 @@ namespace MeshWeaver.Messaging.Serialization; /// -/// Reorders JSON object properties so that $type appears first, which is required -/// by System.Text.Json for polymorphic types with parameterized constructors. -/// Returns the original raw text if no reordering is needed. +/// Deserializes with $type first, which is required by System.Text.Json for polymorphic types +/// with parameterized constructors — reordering only when $type does not already lead. /// internal static class JsonElementNormalizer { private const string TypeDiscriminator = "$type"; - public static string GetNormalizedRawText(JsonElement element) + /// + /// Deserializes to with $type first, + /// straight from UTF-8 — the element's own bytes when $type already leads (no copy at + /// all), else one reordered UTF-8 buffer. Never a UTF-16 string: materialising one per read was + /// 2× the payload in allocations for a value that is immediately parsed + /// back (MeshWeaver#5555). + /// + public static object? Deserialize(JsonElement element, Type type, JsonSerializerOptions options) { - if (element.ValueKind != JsonValueKind.Object) - return element.GetRawText(); - - if (!element.TryGetProperty(TypeDiscriminator, out _)) - return element.GetRawText(); - - // Check if $type is already the first property - using var enumerator = element.EnumerateObject(); - if (!enumerator.MoveNext()) - return element.GetRawText(); - - if (enumerator.Current.Name == TypeDiscriminator) - return element.GetRawText(); // Already first, no work needed + if (!NeedsReorder(element)) + return element.Deserialize(type, options); - // Reorder: write $type first, then all other properties var buffer = new ArrayBufferWriter(); using (var writer = new Utf8JsonWriter(buffer)) { @@ -45,7 +39,16 @@ public static string GetNormalizedRawText(JsonElement element) } writer.WriteEndObject(); } - return System.Text.Encoding.UTF8.GetString(buffer.WrittenSpan); + return JsonSerializer.Deserialize(buffer.WrittenSpan, type, options); + } + + private static bool NeedsReorder(JsonElement element) + { + if (element.ValueKind != JsonValueKind.Object + || !element.TryGetProperty(TypeDiscriminator, out _)) + return false; + using var enumerator = element.EnumerateObject(); + return enumerator.MoveNext() && enumerator.Current.Name != TypeDiscriminator; } } @@ -140,7 +143,7 @@ public override IReadOnlyCollection Read(ref Utf8JsonReader reader, Type type { // Deserialize each element using the proper JsonSerializerOptions // Normalize to ensure $type is first (required for parameterized constructor types) - var item = JsonSerializer.Deserialize(JsonElementNormalizer.GetNormalizedRawText(element), options); + var item = (T?)JsonElementNormalizer.Deserialize(element, typeof(T), options); if (item != null) list.Add(item); } @@ -228,7 +231,7 @@ public override IReadOnlyList Read(ref Utf8JsonReader reader, Type typeToConv var list = new List(); foreach (var element in jsonDoc.RootElement.EnumerateArray()) { - var item = JsonSerializer.Deserialize(JsonElementNormalizer.GetNormalizedRawText(element), options); + var item = (T?)JsonElementNormalizer.Deserialize(element, typeof(T), options); if (item != null) list.Add(item); } @@ -300,7 +303,7 @@ public override IEnumerable Read(ref Utf8JsonReader reader, Type typeToConver var list = new List(); foreach (var element in jsonDoc.RootElement.EnumerateArray()) { - var item = JsonSerializer.Deserialize(JsonElementNormalizer.GetNormalizedRawText(element), options); + var item = (T?)JsonElementNormalizer.Deserialize(element, typeof(T), options); if (item != null) list.Add(item); } diff --git a/test/MeshWeaver.Messaging.Hub.Test/PolymorphicReadAllocationTest.cs b/test/MeshWeaver.Messaging.Hub.Test/PolymorphicReadAllocationTest.cs new file mode 100644 index 0000000000..264941d02b --- /dev/null +++ b/test/MeshWeaver.Messaging.Hub.Test/PolymorphicReadAllocationTest.cs @@ -0,0 +1,124 @@ +using System; +using System.Text; +using System.Text.Json; +using MeshWeaver.Domain; +using MeshWeaver.Fixture; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace MeshWeaver.Messaging.Hub.Test; + +/// +/// MeshWeaver#5555: the [HEAPSTEP] sampler found every multi-hundred-MiB heap step on both +/// portals dominated by System.String ≈ 2 × System.Byte[] plus JsonDocument — +/// a JSON value held once as UTF-8 and once as UTF-16. ObjectPolymorphicConverter.ReadObject +/// did exactly that for every object-typed value: JsonDocument.ParseValue (a byte[] +/// copy + metadata), then GetRawText() (a UTF-16 string, 2× the bytes), then a nested +/// Deserialize(string) — and once PER NESTING LEVEL, because each nested object member +/// re-enters the converter and copies its whole subtree again. +/// +/// The reading here is deterministic: around +/// one deserialization of a value nested levels deep whose leaf holds a +/// -character string. The floor is the leaf string itself (2 bytes per char); +/// the defect costs roughly 3 × that per level. +/// +public class PolymorphicReadAllocationTest(ITestOutputHelper output) : HubTestBase(output) +{ + private const int Depth = 8; + private const int LeafChars = 200_000; + + private record AllocWrapper(object? Inner); + + private record AllocLeaf(string Text); + + private JsonSerializerOptions RegisteredOptions() + { + var host = GetHost(); + var registry = host.ServiceProvider.GetRequiredService(); + registry.WithType(typeof(AllocWrapper), nameof(AllocWrapper)); + registry.WithType(typeof(AllocLeaf), nameof(AllocLeaf)); + return host.JsonSerializerOptions; + } + + private static byte[] NestedJson(bool typeFirst) + { + var sb = new StringBuilder(); + for (var i = 0; i < Depth; i++) + sb.Append(typeFirst + ? $"{{\"$type\":\"{nameof(AllocWrapper)}\",\"inner\":" + : $"{{\"inner\":"); + sb.Append($"{{\"$type\":\"{nameof(AllocLeaf)}\",\"text\":\"").Append('a', LeafChars).Append("\"}"); + for (var i = 0; i < Depth; i++) + sb.Append(typeFirst ? "}" : $",\"$type\":\"{nameof(AllocWrapper)}\"}}"); + return Encoding.UTF8.GetBytes(sb.ToString()); + } + + private static void AssertChain(object? value) + { + for (var i = 0; i < Depth; i++) + { + value.Should().BeOfType($"level {i} carries a registered $type"); + value = ((AllocWrapper)value!).Inner; + } + value.Should().BeOfType().Which.Text.Length.Should().Be(LeafChars); + } + + [Fact] + public void NestedTypedRead_AllocatesTheLeafOnce_NotACopyPerLevel() + { + var options = RegisteredOptions(); + var utf8 = NestedJson(typeFirst: true); + + // Warm-up: type-info resolution and pooled buffers are one-time costs, not per-read ones. + AssertChain(JsonSerializer.Deserialize(utf8, options)); + + var before = GC.GetAllocatedBytesForCurrentThread(); + var result = JsonSerializer.Deserialize(utf8, options); + var allocated = GC.GetAllocatedBytesForCurrentThread() - before; + + AssertChain(result); + var leafBytes = 2L * LeafChars; + Output.WriteLine($"allocated {allocated:N0} bytes for a {leafBytes:N0}-byte leaf at depth {Depth}"); + allocated.Should().BeLessThan(2 * leafBytes, + "a typed read materialises the leaf string once; a JsonDocument copy plus a UTF-16 " + + "string per nesting level is the String ≈ 2×Byte[] + JsonDocument churn of #5555"); + } + + /// + /// The general path ($type not first — needs reordering) must stay correct and must not + /// build a UTF-16 string either; it still pays one JsonDocument per level, which is why the + /// bound is looser than the direct path's. + /// + [Fact] + public void NestedTypedRead_TypeNotFirst_StillTypedAndStringFree() + { + var options = RegisteredOptions(); + var utf8 = NestedJson(typeFirst: false); + + AssertChain(JsonSerializer.Deserialize(utf8, options)); + + var before = GC.GetAllocatedBytesForCurrentThread(); + var result = JsonSerializer.Deserialize(utf8, options); + var allocated = GC.GetAllocatedBytesForCurrentThread() - before; + + AssertChain(result); + var leafBytes = 2L * LeafChars; + Output.WriteLine($"allocated {allocated:N0} bytes for a {leafBytes:N0}-byte leaf at depth {Depth} ($type last)"); + allocated.Should().BeLessThan((Depth + 2) * leafBytes, + "without the UTF-16 round trip each level costs about one UTF-8 copy (half the leaf's " + + "UTF-16 size) plus a reorder buffer, not the ~3× the string round trip cost"); + } + + /// A registered type whose JSON no longer fits is preserved as raw JSON, not thrown. + [Fact] + public void RegisteredTypeThatDoesNotFit_IsPreservedAsRawJson() + { + var options = RegisteredOptions(); + var json = $"{{\"$type\":\"{nameof(AllocLeaf)}\",\"text\":{{\"not\":\"a string\"}}}}"; + + var result = JsonSerializer.Deserialize(json, options); + + result.Should().BeOfType().Which.GetProperty("text").GetProperty("not").GetString() + .Should().Be("a string"); + } +} From 319acdeb43316fcbb1104d9b62195c0feed4b5df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Sat, 26 Sep 2026 20:28:25 +0200 Subject: [PATCH 2/2] review: inline the metadata-name check (no static array); pin $type-first + reference metadata to the stripping path Negative control: with the probe's metadata check disabled, all four TypeFirstWithReferenceMetadata_IsStrippedAndTyped cases go red. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ObjectPolymorphicConverter.cs | 10 ++++----- .../PolymorphicReadAllocationTest.cs | 22 ++++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/MeshWeaver.Messaging.Hub/Serialization/ObjectPolymorphicConverter.cs b/src/MeshWeaver.Messaging.Hub/Serialization/ObjectPolymorphicConverter.cs index 62b99309b3..6978a231c6 100644 --- a/src/MeshWeaver.Messaging.Hub/Serialization/ObjectPolymorphicConverter.cs +++ b/src/MeshWeaver.Messaging.Hub/Serialization/ObjectPolymorphicConverter.cs @@ -191,8 +191,6 @@ or InvalidOperationException return cleanedElement.Clone(); } - private static readonly string[] MetadataPropertyNames = ["$id", "$ref", "$values", "$defs"]; - /// /// Deserializes an object whose FIRST property is a registered $type directly from the /// reader — no , no string. Declines (returns false, reader untouched) @@ -220,9 +218,11 @@ private bool TryReadRegisteredTypeFromReader( while (probe.Read() && probe.TokenType == JsonTokenType.PropertyName) { - foreach (var metadata in MetadataPropertyNames) - if (probe.ValueTextEquals(metadata)) - return false; + // The reference-metadata names StripMetadataProperties removes: those shapes take the + // general path so the strip still applies. + if (probe.ValueTextEquals("$id") || probe.ValueTextEquals("$ref") + || probe.ValueTextEquals("$values") || probe.ValueTextEquals("$defs")) + return false; if (!probe.Read() || !probe.TrySkip()) return false; } diff --git a/test/MeshWeaver.Messaging.Hub.Test/PolymorphicReadAllocationTest.cs b/test/MeshWeaver.Messaging.Hub.Test/PolymorphicReadAllocationTest.cs index 264941d02b..e62f4f3a11 100644 --- a/test/MeshWeaver.Messaging.Hub.Test/PolymorphicReadAllocationTest.cs +++ b/test/MeshWeaver.Messaging.Hub.Test/PolymorphicReadAllocationTest.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Text; using System.Text.Json; using MeshWeaver.Domain; @@ -109,6 +109,26 @@ public void NestedTypedRead_TypeNotFirst_StillTypedAndStringFree() + "UTF-16 size) plus a reorder buffer, not the ~3× the string round trip cost"); } + /// + /// A $type-first object that also carries reference metadata must NOT take the direct + /// path: the general path strips $id/$ref/$values/$defs before the + /// typed read, and the direct path does not. + /// + [Theory] + [InlineData("$id")] + [InlineData("$ref")] + [InlineData("$values")] + [InlineData("$defs")] + public void TypeFirstWithReferenceMetadata_IsStrippedAndTyped(string metadata) + { + var options = RegisteredOptions(); + var json = $"{{\"$type\":\"{nameof(AllocLeaf)}\",\"{metadata}\":\"1\",\"text\":\"kept\"}}"; + + var result = JsonSerializer.Deserialize(json, options); + + result.Should().BeOfType().Which.Text.Should().Be("kept"); + } + /// A registered type whose JSON no longer fits is preserved as raw JSON, not thrown. [Fact] public void RegisteredTypeThatDoesNotFit_IsPreservedAsRawJson()