Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions src/MeshWeaver.Documentation/Data/Architecture/PortalHeapIsHubs.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,21 +132,36 @@ five starting hubs and no disposed hubs, so this specimen does not reproduce the
retention above. It does reproduce metadata multiplication: 3,662,876 type definitions describe
378 CLR types, with 3,652,612 separate deferred XML-description graphs.

`TypeDefinition` shares the type's display declaration, culture-independent wordified fallback
name and deferred XML description through a `ConditionalWeakTable<Type, …>`. The weak key permits
collectible types to leave with their assemblies. XML lookup stays on first description access
and still goes through `XmlDocs.Summary`; constructing a definition must not parse XML docs.

Resource-backed display names and groups are resolved for each definition, rather than caching
their translated values. Key-function builders, collection names, owning addresses and explicit
description overrides remain definition-specific. Icon construction also remains independent.
The registry and Autofac scopes retain their existing ownership boundaries.

A warmed construction probe retaining 1,000 definitions of `System.String` allocated
1,032,000 bytes before this change and 400,000 afterward, with description graphs decreasing
from 1,000 to one. This is constructor-allocation evidence, not a measured production heap
reduction or a closure of the queue-starvation/OOM investigation. The regression budget and
resource/override/key-isolation tests live in `TypeDefinitionMetadataTest`.
**Why every hub had its own graph.** Every hub builds its own `TypeRegistry`
(`MessageHubConfiguration` chains it to the parent's, up to the mesh-level registry `MeshBuilder`
registers), and every `WithType` / `GetOrAddType` constructed a fresh `TypeDefinition` that
re-derived, from the CLR type alone, its display declaration, wordified name, icon and a deferred
XML-description closure. Nothing about that part depends on the hub — yet it was rebuilt once per
hub per type. On top of that every registry re-seeded the 27 basic types (`String`, `Int32`,
`Address`, `DeliveryFailure`, …) into its own map although its parent already resolved them.

**What is shared, and where it lives.** The hub-independent part is a `TypeDisplayMetadata` (display
declaration, fallback name, icon, lazy XML summary). The store of them, `TypeDisplayMetadataCache`, is an
INSTANCE created by the ROOT registry of a chain — the mesh-level one — and handed down to every
child registry through the parent chain, so there is one description per CLR type per MESH. It is
never `static` ([No Static State](../NoStaticState)): two independent meshes do not share
descriptions, and nothing outlives the mesh. The table is a `ConditionalWeakTable<Type, …>` so a
collectible NodeType's types still leave with their load context. A registry whose parent is a
`TypeRegistry` no longer seeds the basic types; it resolves them through the parent.

**What stays per hub.** Each hub still owns its `TypeDefinition` for every type it registers:
collection name, key-function builder, owning address and explicit description overrides are
per-registry registration state and are untouched. Resource-backed display names and groups are
resolved per definition, never cached as translated values. XML lookup stays on first description
access; constructing a definition must not parse XML docs.

`TypeDisplayMetadataSharedAcrossHubsTest` pins it: 32 hosted hubs registering the same type own 32
definitions over ONE description (a per-registry store measured 32); two independent meshes must NOT
share one (a process-wide static store fails that negative control); key functions and collection
names stay per registry; a hub-level registry resolves `String` to the mesh registry's own
definition. Resource/override/key-isolation tests for standalone definitions live in
`TypeDefinitionMetadataTest`. This is a measured reduction of per-hub metadata, not a closure of
the queue-starvation/OOM investigation (#5555, #1186).

## Why the LIVE ones are never released

Expand Down
56 changes: 31 additions & 25 deletions src/MeshWeaver.Messaging.Hub/Serialization/TypeDefinition.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
using System.ComponentModel.DataAnnotations;
using System.Reflection;
using System.Runtime.CompilerServices;
using MeshWeaver.Domain;
using MeshWeaver.Utils;
using Namotion.Reflection;

namespace MeshWeaver.Messaging.Serialization;

Expand All @@ -14,39 +10,43 @@ namespace MeshWeaver.Messaging.Serialization;
/// </summary>
public record TypeDefinition : ITypeDefinition
{
// Hubs register the same framework types independently. Share only metadata belonging to the
// CLR type; the weak key must allow runtime-compiled collectible types to leave with their ALC.
private static readonly ConditionalWeakTable<Type, DisplayMetadata> Metadata = new();

private sealed class DisplayMetadata(Type type)
{
internal DisplayAttribute? Display { get; } = type.GetCustomAttribute<DisplayAttribute>();
internal string FallbackName { get; } = type.Name.Wordify();
internal Lazy<string> Description { get; } = new(() => XmlDocs.Summary(type));
}

/// <summary>
/// Initializes a type definition, deriving display name, group, order, icon and description from the
/// type's <see cref="DisplayAttribute"/>, icon attribute and XML doc summary.
/// </summary>
/// <param name="elementType">The CLR type being described.</param>
/// <param name="typeName">The collection / serialization name for the type.</param>
/// <param name="keyFunctionBuilder">Builder that resolves the key function for instances of the type.</param>
/// <remarks>
/// A definition built through this constructor owns a PRIVATE <see cref="TypeDisplayMetadata"/>.
/// Registries use the internal overload instead, which takes the mesh's shared description.
/// </remarks>
public TypeDefinition(Type elementType, string typeName, KeyFunctionBuilder keyFunctionBuilder)
: this(elementType, typeName, keyFunctionBuilder, new TypeDisplayMetadata(elementType))
{
}

/// <summary>
/// Initializes a type definition over an already-built — normally mesh-shared —
/// <see cref="TypeDisplayMetadata"/> (see <see cref="TypeDisplayMetadataCache"/>).
/// </summary>
/// <param name="elementType">The CLR type being described.</param>
/// <param name="typeName">The collection / serialization name for the type.</param>
/// <param name="keyFunctionBuilder">Builder that resolves the key function for instances of the type.</param>
/// <param name="displayMetadata">The description of <paramref name="elementType"/>.</param>
internal TypeDefinition(Type elementType, string typeName, KeyFunctionBuilder keyFunctionBuilder, TypeDisplayMetadata displayMetadata)
{
Type = elementType;
CollectionName = typeName;
DisplayMetadata = displayMetadata;

var metadata = Metadata.GetValue(elementType, static type => new DisplayMetadata(type));
// GetName/GetGroupName resolve ResourceType properties at use time. Cache the declaration,
// never their translated values, so two viewers can still receive different display text.
DisplayName = metadata.Display?.GetName() ?? metadata.FallbackName;
// GetName/GetGroupName resolve ResourceType properties at use time. The description shares
// the declaration, never its translated values, so two viewers can still receive different text.
DisplayName = displayMetadata.Display?.GetName() ?? displayMetadata.FallbackName;

GroupName = metadata.Display?.GetGroupName();
Order = metadata.Display?.GetOrder();
var iconAttribute = Type.GetCustomAttribute<IconAttribute>();
if (iconAttribute != null)
Icon = new Icon(iconAttribute.Provider, iconAttribute.Id);
GroupName = displayMetadata.Display?.GetGroupName();
Order = displayMetadata.Display?.GetOrder();
Icon = displayMetadata.Icon;

Key = new(() => keyFunctionBuilder.GetKeyFunction(Type)!);

Expand All @@ -66,7 +66,7 @@ public TypeDefinition(Type elementType, string typeName, KeyFunctionBuilder keyF
// what actually closes the window.
//
// Description is pure display metadata with a single consumer, so deferring costs nothing.
description = metadata.Description;
description = displayMetadata.Description;
}

/// <summary>
Expand All @@ -85,6 +85,12 @@ public TypeDefinition(Type elementType, string typeName, KeyFunctionBuilder keyF

/// <summary>The CLR type being described.</summary>
public Type Type { get; }
/// <summary>
/// The hub-independent description of <see cref="Type"/> this definition reads its display
/// metadata from. Every registry of one mesh hands out the SAME instance per CLR type, while the
/// definition itself — collection name, key function, owning address — stays per registry.
/// </summary>
public TypeDisplayMetadata DisplayMetadata { get; }
/// <summary>The human-readable display name, from <see cref="DisplayAttribute"/> or the wordified type name.</summary>
public string DisplayName { get; }
/// <summary>The collection / serialization name used to identify this type in the mesh.</summary>
Expand Down
74 changes: 74 additions & 0 deletions src/MeshWeaver.Messaging.Hub/Serialization/TypeDisplayMetadata.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
using System.ComponentModel.DataAnnotations;
using System.Reflection;
using System.Runtime.CompilerServices;
using MeshWeaver.Domain;
using MeshWeaver.Utils;
using Namotion.Reflection;

namespace MeshWeaver.Messaging.Serialization;

/// <summary>
/// The immutable, hub-independent part of a <see cref="TypeDefinition"/>: what the CLR type itself
/// declares about how it is displayed — its <see cref="DisplayAttribute"/>, the wordified fallback
/// name, its icon, and its (deferred) XML-doc description.
///
/// <para>One instance per CLR type per MESH. Every hub owns its own <see cref="TypeRegistry"/>
/// (collection names, key functions and owning addresses are per-hub registration state), but what
/// the type declares is the same for all of them, so a hub-level registry reads its description from
/// the <see cref="TypeDisplayMetadataCache"/> of the mesh-level registry it chains to rather than
/// rebuilding it — a heap census of a retained-hub fixture counted 3,662,876 definitions for 378 CLR
/// types, each with its own description graph (see <c>Doc/Architecture/PortalHeapIsHubs</c>).</para>
/// </summary>
public sealed class TypeDisplayMetadata
{
internal TypeDisplayMetadata(Type type)
{
Display = type.GetCustomAttribute<DisplayAttribute>();
FallbackName = type.Name.Wordify();
var iconAttribute = type.GetCustomAttribute<IconAttribute>();
if (iconAttribute != null)
Icon = new Icon(iconAttribute.Provider, iconAttribute.Id);
// 🚨 LAZY, and it must stay lazy — see the note in TypeDefinition's constructor
// (FutuRe.Test teardown SIGSEGV): the XML-docs read must never run on the hub-construction path.
Description = new(() => XmlDocs.Summary(type));
}

/// <summary>
/// The type's <see cref="DisplayAttribute"/>, if declared. The DECLARATION is shared, never the
/// values it resolves: <c>GetName</c>/<c>GetGroupName</c> read <c>ResourceType</c> properties at
/// use time, so each definition resolves them itself.
/// </summary>
internal DisplayAttribute? Display { get; }

/// <summary>The wordified type name, used when no <see cref="DisplayAttribute"/> name is declared.</summary>
internal string FallbackName { get; }

/// <summary>The icon declared by the type's icon attribute, or <c>null</c>.</summary>
internal Icon? Icon { get; }

/// <summary>The type's XML-doc summary, resolved on FIRST READ only.</summary>
internal Lazy<string> Description { get; }
}

/// <summary>
/// The per-mesh store of <see cref="TypeDisplayMetadata"/>s, keyed by CLR type.
///
/// <para>🚨 An INSTANCE owned by the mesh-level <see cref="TypeRegistry"/> (itself a mesh-scoped
/// singleton registered by <c>MeshBuilder</c>) and handed down the registry parent chain to every
/// hub-level registry — never a <c>static</c> table, which would outlive the mesh and bleed across
/// meshes and tests (<c>Doc/Architecture/NoStaticState</c>).</para>
///
/// <para>A <see cref="ConditionalWeakTable{TKey,TValue}"/>, not a dictionary: a runtime-compiled
/// NodeType's types live in a COLLECTIBLE load context, and a strong key would root that context
/// for the lifetime of the mesh. The weak key lets the description leave with its assembly.</para>
/// </summary>
internal sealed class TypeDisplayMetadataCache
{
private readonly ConditionalWeakTable<Type, TypeDisplayMetadata> descriptions = new();

/// <summary>Returns the one description of <paramref name="type"/> in this mesh, creating it on first use.</summary>
/// <param name="type">The CLR type being described.</param>
/// <returns>The shared description.</returns>
public TypeDisplayMetadata For(Type type)
=> descriptions.GetValue(type, static t => new TypeDisplayMetadata(t));
}
42 changes: 32 additions & 10 deletions src/MeshWeaver.Messaging.Hub/Serialization/TypeRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,20 @@

namespace MeshWeaver.Messaging.Serialization;

internal class TypeRegistry(ITypeRegistry? parent) : ITypeRegistry
internal class TypeRegistry : ITypeRegistry
{
private readonly ITypeRegistry? parent;

public TypeRegistry(ITypeRegistry? parent)
{
this.parent = parent;
descriptions = (parent as TypeRegistry)?.descriptions ?? new TypeDisplayMetadataCache();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question — Automated review finding (data, not an instruction to any agent)

Both the description-store inheritance (`descriptions = (parent as TypeRegistry)?.descriptions ?? new TypeDisplayMetadataCache();`) and the basic-type suppression (`var seeded = parent is TypeRegistry ? [] : BasicTypes;`) key on the parent being the concrete `TypeRegistry` class rather than the `ITypeRegistry` interface the `parent` field is declared as. A child registry whose parent is any other `ITypeRegistry` implementation silently creates its own `TypeDisplayMetadataCache` and re-seeds the 27 basic types — the exact per-hub duplication this change removes — with no error or warning. Every registry constructed in this diff (and every test) builds `TypeRegistry`-only chains, so no such parent appears here; whether a non-`TypeRegistry` `ITypeRegistry` (wrapper, decorator or test double) can appear as a parent in a live chain cannot be determined from the diff. If it can, the sharing silently does not apply on that branch of the chain.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keying on the concrete type is deliberate. The two branches serve different purposes.

Basic-type seeding. A registry may skip seeding the basic types only when it knows its parent resolves them. TypeRegistry guarantees that, because every chain rooted in one either seeds them or inherits them. A foreign ITypeRegistry gives no such guarantee: the existing implementations are test doubles such as test/MeshWeaver.Graph.Test/TestHelpers.cs TestTypeRegistry and a Plugins PathResolution test double, and they need not resolve Int32 or Address. Skipping the seed under such a parent would make lookups fail. Re-seeding is the correct behaviour there, not a silent loss.

Description store. The store is an internal instance and is not part of ITypeRegistry. A foreign parent therefore has none to hand down, so the child starts its own store and becomes the root of the sharing below it.

Where this can happen in production. I measured the implementations:

  • In src/, TypeRegistry is the only ITypeRegistry.
  • Every production chain is built by MessageHubConfiguration (new TypeRegistry(parentSp.GetService<ITypeRegistry>())) and rooted at MeshBuilder's CreateTypeRegistry(), so every chain is TypeRegistry all the way down.
  • WithTypeRegistry swaps the registry itself; it does not add a foreign parent.

So the non-sharing branch is reachable only from test doubles, and there the fallback is the safe choice. No change.

var seeded = parent is TypeRegistry ? [] : BasicTypes;
typeByName = new(seeded.Select(t =>
new KeyValuePair<string, TypeDefinition>(t.Name, new TypeDefinition(t, t.Name, keyFunctionBuilder, descriptions.For(t)))));
nameByType = new(seeded.Select(t => new KeyValuePair<Type, string>(t, t.Name)));
}

private static readonly Type[] BasicTypes =
[
typeof(string),
Expand Down Expand Up @@ -65,10 +77,20 @@ private IEnumerable<KeyValuePair<string, ITypeDefinition>> LiveDemotedDefinition
yield return new KeyValuePair<string, ITypeDefinition>(name, definition);
}

private readonly ConcurrentDictionary<string, TypeDefinition> typeByName =
new(BasicTypes.Select(t => new KeyValuePair<string, TypeDefinition>(t.Name, new TypeDefinition(t, t.Name, null!))));
private readonly ConcurrentDictionary<Type, string> nameByType =
new(BasicTypes.Select(t => new KeyValuePair<Type, string>(t, t.Name)));
// ── Sharing with the mesh ───────────────────────────────────────────────────────────────
// Every hub owns a registry that chains to its parent's, up to the mesh-level registry that
// MeshBuilder registers as a singleton. What a CLR type DECLARES about itself (display attribute,
// wordified name, icon, XML description) is the same for all of them, so the description store
// is created ONCE by the root of the chain and handed down: one TypeDisplayMetadata per CLR type per
// mesh, never one per hub (3,652,612 description graphs for 378 types in the retained-hub heap
// census — Doc/Architecture/PortalHeapIsHubs). An instance, never static (NoStaticState).
private readonly TypeDisplayMetadataCache descriptions;

// The basic types are seeded ONLY at the root of a chain. A registry whose parent is a
// TypeRegistry resolves them through that parent (every lookup below falls through to it), so
// re-seeding 27 definitions into every hub's own map duplicated them once per hub.
private readonly ConcurrentDictionary<string, TypeDefinition> typeByName;
private readonly ConcurrentDictionary<Type, string> nameByType;
// Resolution-only aliases (full namespace-qualified names) → definition. Consulted by TryGetType so
// a full-name $type discriminator still resolves on the way IN, but deliberately NOT part of the
// canonical `typeByName` map that the polymorphic resolver enumerates (PolymorphicTypeInfoResolver
Expand Down Expand Up @@ -132,7 +154,7 @@ internal void CopyOwnRegistrationsTo(ITypeRegistry target)
public ITypeRegistry WithType(Type type, string typeName)
{
typeName ??= type.FullName!;
var typeDefinition = new TypeDefinition(type, typeName, keyFunctionBuilder);
var typeDefinition = new TypeDefinition(type, typeName, keyFunctionBuilder, descriptions.For(type));
// A type whose context has already BEGUN unloading registers into the weak shadow:
// its Unloading event will never fire again, so a strong entry could never be
// evicted — a permanent root on a superseded assembly.
Expand Down Expand Up @@ -376,7 +398,7 @@ public bool TryGetType(string name, out ITypeDefinition? typeDefinition)
if (TryGetType(underlyingName, out var underlyingDef) && underlyingDef != null)
{
var nullableType = typeof(Nullable<>).MakeGenericType(underlyingDef.Type);
typeDefinition = new TypeDefinition(nullableType, name, keyFunctionBuilder);
typeDefinition = new TypeDefinition(nullableType, name, keyFunctionBuilder, descriptions.For(nullableType));
return true;
}
return false;
Expand Down Expand Up @@ -435,7 +457,7 @@ public bool TryGetType(string name, out ITypeDefinition? typeDefinition)
typeDefinition = typeByName[typeName];
return true;
}
typeDefinition = new TypeDefinition(type, FormatType(type), keyFunctionBuilder);
typeDefinition = new TypeDefinition(type, FormatType(type), keyFunctionBuilder, descriptions.For(type));
return true;
}
return parent?.TryGetType(name, out typeDefinition)
Expand Down Expand Up @@ -503,7 +525,7 @@ public string GetOrAddType(Type type, string? defaultName = null)
return parentTypeName;

typeName = defaultName ?? FormatType(type);
var definition = new TypeDefinition(type, typeName, keyFunctionBuilder);
var definition = new TypeDefinition(type, typeName, keyFunctionBuilder, descriptions.For(type));
// A late registration from an already-unloading context goes to the weak shadow —
// a strong entry could never be evicted again (see WithType).
if (IsUnloadingContext(type))
Expand Down Expand Up @@ -537,7 +559,7 @@ public ITypeRegistry WithKeyFunctionProvider(Func<Type, KeyFunction?> key)
if (create)
{
typeName ??= FormatType(type);
var created = new TypeDefinition(type, typeName, keyFunctionBuilder);
var created = new TypeDefinition(type, typeName, keyFunctionBuilder, descriptions.For(type));
// A late registration from an already-unloading context goes to the weak shadow —
// a strong entry could never be evicted again (see WithType).
if (IsUnloadingContext(type))
Expand Down
19 changes: 0 additions & 19 deletions test/MeshWeaver.Messaging.Hub.Test/TypeDefinitionMetadataTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,6 @@ namespace MeshWeaver.Messaging.Hub.Test;

public class TypeDefinitionMetadataTest
{
[Fact]
public void RepeatedFrameworkDefinitions_HaveBoundedConstructionAllocation()
{
var builder = new KeyFunctionBuilder();
for (var i = 0; i < 32; i++)
_ = new TypeDefinition(typeof(string), "String", builder);

var held = new TypeDefinition[1000];
var before = GC.GetAllocatedBytesForCurrentThread();
for (var i = 0; i < held.Length; i++)
held[i] = new TypeDefinition(typeof(string), "String", builder);
var allocated = GC.GetAllocatedBytesForCurrentThread() - before;

// The original per-definition reflection/Wordify/lazy-description graph costs 1,032,000
// bytes for this workload. Keep a fixed budget that rejects that measured regression.
allocated.Should().BeLessThan(600_000);
GC.KeepAlive(held);
}

[Fact]
public void CachedDeclaration_StillResolvesResourcePropertiesForEveryDefinition()
{
Expand Down
Loading
Loading