From f4e035445c53f1bf44ad22970c313d819711e083 Mon Sep 17 00:00:00 2001 From: David Kean Date: Thu, 9 Apr 2026 13:49:47 +1000 Subject: [PATCH 1/5] Replace ImmutableDictionary with Dictionary in LazyMetadataWrapper ImmutableDictionary allocates SortedInt32KeyNode tree nodes on construction and a new dictionary on every SetItem call. Dictionary avoids both. The old code wrote substituted values back via ImmutableDictionary.SetItem, which was structurally thread-safe (new dict + atomic reference swap). With Dictionary, concurrent writes would corrupt internal state. The write-back is removed; substitution cost without caching is negligible since TypeRef.Resolve() already caches its result and Enum.ToObject is trivial. --- .../LazyMetadataWrapper.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.VisualStudio.Composition/LazyMetadataWrapper.cs b/src/Microsoft.VisualStudio.Composition/LazyMetadataWrapper.cs index 4b78ec5ae..53923aa72 100644 --- a/src/Microsoft.VisualStudio.Composition/LazyMetadataWrapper.cs +++ b/src/Microsoft.VisualStudio.Composition/LazyMetadataWrapper.cs @@ -32,9 +32,9 @@ internal class LazyMetadataWrapper : ExportProvider.IMetadataDictionary /// The underlying metadata, which may be partially translated since value translation may choose /// to persist the translated result. /// - protected ImmutableDictionary underlyingMetadata; + protected Dictionary underlyingMetadata; - internal LazyMetadataWrapper(ImmutableDictionary metadata, Direction direction, Resolver resolver) + internal LazyMetadataWrapper(Dictionary metadata, Direction direction, Resolver resolver) { Requires.NotNull(metadata, nameof(metadata)); Requires.NotNull(resolver, nameof(resolver)); @@ -253,7 +253,7 @@ internal static bool TryGetLoadSafeValueTypeRef(IReadOnlyDictionary newMetadata) { - return new LazyMetadataWrapper(newMetadata.ToImmutableDictionary(), oldVersion.direction, this.resolver); + return new LazyMetadataWrapper(newMetadata.ToDictionary(), oldVersion.direction, this.resolver); } protected object? SubstituteValueIfRequired(string key, object? value) @@ -265,13 +265,13 @@ protected virtual LazyMetadataWrapper Clone(LazyMetadataWrapper oldVersion, IRea return null; } - value = this.SubstituteValueIfRequired(value); - - // Update our metadata dictionary with the substitution to avoid - // the translation costs next time. - this.underlyingMetadata = this.underlyingMetadata.SetItem(key, value); - - return value; + // Note: we intentionally do NOT write the substituted value back into the dictionary. + // The old ImmutableDictionary-backed implementation called SetItem here, which was + // structurally thread-safe (it returned a new dictionary and assigned the reference + // atomically). With Dictionary, concurrent writes would corrupt the internal state. + // The substitution cost without caching is negligible: TypeRef.Resolve() caches its + // result in a field, and Enum.ToObject is trivial. + return this.SubstituteValueIfRequired(value); } protected virtual object SubstituteValueIfRequired(object value) From 85441e1fc35e90284d54d9413c6c6dc46ceb6a56 Mon Sep 17 00:00:00 2001 From: David Kean Date: Thu, 9 Apr 2026 13:51:13 +1000 Subject: [PATCH 2/5] Use Dictionary in ReadMetadata/WriteMetadata and cache empty metadata Build metadata into Dictionary directly instead of ImmutableDictionary.Builder. Return ImmutableDictionary.Empty singleton for empty import metadata (majority of imports have no metadata), avoiding a LazyMetadataWrapper allocation. Add ToDictionary extension for IReadOnlyDictionary since the BCL Dictionary constructor does not accept IReadOnlyDictionary on netstandard2.0. --- .../Configuration/SerializationContextBase.cs | 35 +++++++------------ .../Utilities.cs | 16 +++++++++ 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs b/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs index a4ea5b851..81dccd950 100644 --- a/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs +++ b/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs @@ -49,7 +49,6 @@ internal abstract partial class SerializationContextBase : IDisposable private readonly Func readObjectDelegate; private readonly Func readTypeRefDelegate; - private readonly ImmutableDictionary.Builder metadataBuilder = ImmutableDictionary.CreateBuilder(); private readonly Stack.Builder> typeRefBuilders = new Stack.Builder>(); private readonly byte[] guidBuffer = new byte[128 / 8]; @@ -59,6 +58,8 @@ internal abstract partial class SerializationContextBase : IDisposable private static readonly object BoxedTrue = true; private static readonly object BoxedFalse = false; + private static readonly IReadOnlyDictionary EmptyMetadata = ImmutableDictionary.Empty; + internal SerializationContextBase(BinaryReader reader, Resolver resolver) { Requires.NotNull(reader, nameof(reader)); @@ -801,7 +802,7 @@ protected void Write(IReadOnlyDictionary metadata) // implicitly resolving TypeRefs to Types which is undesirable. metadata = LazyMetadataWrapper.TryUnwrap(metadata); - serializedMetadata = new LazyMetadataWrapper(metadata.ToImmutableDictionary(), LazyMetadataWrapper.Direction.ToSubstitutedValue, this.Resolver); + serializedMetadata = new LazyMetadataWrapper(metadata.ToDictionary(), LazyMetadataWrapper.Direction.ToSubstitutedValue, this.Resolver); foreach (var entry in serializedMetadata) { @@ -815,32 +816,22 @@ protected void Write(IReadOnlyDictionary metadata) { using (this.Trace("Metadata")) { - // PERF TIP: if ReadMetadata shows up on startup perf traces, - // we could simply read the blob containing the metadata into a byte[] - // and defer actually deserializing it until such time as the metadata - // is actually required. - // We might do this with minimal impact to other code by implementing - // IReadOnlyDictionary ourselves such that on the first - // access of any of its contents, we'll do a just-in-time deserialization, - // and perhaps only of the requested values. uint count = this.ReadCompressedUInt(); - var metadata = ImmutableDictionary.Empty; - if (count > 0) + if (count == 0) { - var builder = this.metadataBuilder; // reuse builder to save on GC pressure - for (int i = 0; i < count; i++) - { - string? key = this.ReadString(); - object? value = this.ReadObject(); - builder.Add(key, value); - } + return EmptyMetadata; + } - metadata = builder.ToImmutable(); - builder.Clear(); // clean up for the next user. + var dictionary = new Dictionary((int)count); + for (int i = 0; i < count; i++) + { + string? key = this.ReadString(); + object? value = this.ReadObject(); + dictionary.Add(key, value); } - return new LazyMetadataWrapper(metadata, LazyMetadataWrapper.Direction.ToOriginalValue, this.Resolver); + return new LazyMetadataWrapper(dictionary, LazyMetadataWrapper.Direction.ToOriginalValue, this.Resolver); } } diff --git a/src/Microsoft.VisualStudio.Composition/Utilities.cs b/src/Microsoft.VisualStudio.Composition/Utilities.cs index 36e7b208e..06ef02786 100644 --- a/src/Microsoft.VisualStudio.Composition/Utilities.cs +++ b/src/Microsoft.VisualStudio.Composition/Utilities.cs @@ -228,5 +228,21 @@ internal static Dictionary ToDictionary(this IEnumer return dictionary; } + + /// + /// Creates a from an . + /// The BCL Dictionary constructor does not accept IReadOnlyDictionary on netstandard2.0. + /// + internal static Dictionary ToDictionary(this IReadOnlyDictionary source) + where TKey : notnull + { + var dictionary = new Dictionary(source.Count); + foreach (var kvp in source) + { + dictionary.Add(kvp.Key, kvp.Value); + } + + return dictionary; + } } } From 8f107e0be3495444540080c86bb1cd2fd69dc4f9 Mon Sep 17 00:00:00 2001 From: David Kean Date: Thu, 9 Apr 2026 13:51:56 +1000 Subject: [PATCH 3/5] Reduce RuntimeImport per-instance size and cache lazy factories Remove 5 cached fields from RuntimeImport that read already-cached TypeRef.ResolvedType (IsLazy, ImportingSiteElementType, MetadataType, isMetadataTypeInitialized, NullableBool). Retain ImportingMember and ImportingParameter caches since they involve CLR metadata resolution. Move lazy factory delegate creation into a static ConcurrentDictionary cache in LazyServices, keyed by (exportType, metadataViewType). This shares delegates across all imports with the same Lazy or Lazy signature, eliminating per-import MakeGenericMethod and CreateDelegate calls. --- .../LazyServices.cs | 18 +++---- .../RuntimeComposition.cs | 51 ++++--------------- 2 files changed, 18 insertions(+), 51 deletions(-) diff --git a/src/Microsoft.VisualStudio.Composition/LazyServices.cs b/src/Microsoft.VisualStudio.Composition/LazyServices.cs index a5946d902..3dfaae5da 100644 --- a/src/Microsoft.VisualStudio.Composition/LazyServices.cs +++ b/src/Microsoft.VisualStudio.Composition/LazyServices.cs @@ -4,6 +4,7 @@ namespace Microsoft.VisualStudio.Composition { using System; + using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -19,6 +20,7 @@ internal static class LazyServices { private static readonly MethodInfo CreateStronglyTypedLazyOfTMValue = typeof(LazyServices).GetTypeInfo().GetMethod("CreateStronglyTypedLazyOfTM", BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo CreateStronglyTypedLazyOfTValue = typeof(LazyServices).GetTypeInfo().GetMethod("CreateStronglyTypedLazyOfT", BindingFlags.NonPublic | BindingFlags.Static)!; + private static readonly ConcurrentDictionary<(Type ExportType, Type? MetadataViewType), Func, object, object>> LazyFactoryCache = new(); private static readonly string Lazy1FullName = typeof(Lazy<>).FullName!; private static readonly string Lazy2FullName = typeof(Lazy<,>).FullName!; @@ -79,17 +81,15 @@ internal static Lazy FromValue(T value) /// A function that takes a Func{object} value factory and metadata, and produces a Lazy{T, TMetadata} instance. internal static Func, object, object> CreateStronglyTypedLazyFactory(Type? exportType, Type? metadataViewType) { - MethodInfo genericMethod; - if (metadataViewType != null) + var key = (exportType ?? DefaultExportedValueType, metadataViewType); + return LazyFactoryCache.GetOrAdd(key, static k => { - genericMethod = CreateStronglyTypedLazyOfTMValue.MakeGenericMethod(exportType ?? DefaultExportedValueType, metadataViewType); - } - else - { - genericMethod = CreateStronglyTypedLazyOfTValue.MakeGenericMethod(exportType ?? DefaultExportedValueType); - } + MethodInfo genericMethod = k.MetadataViewType != null + ? CreateStronglyTypedLazyOfTMValue.MakeGenericMethod(k.ExportType, k.MetadataViewType) + : CreateStronglyTypedLazyOfTValue.MakeGenericMethod(k.ExportType); - return (Func, object, object>)genericMethod.CreateDelegate(typeof(Func, object, object>)); + return (Func, object, object>)genericMethod.CreateDelegate(typeof(Func, object, object>)); + }); } internal static Func AsFunc(this Lazy lazy) diff --git a/src/Microsoft.VisualStudio.Composition/RuntimeComposition.cs b/src/Microsoft.VisualStudio.Composition/RuntimeComposition.cs index d60f37e7b..a4becfe72 100644 --- a/src/Microsoft.VisualStudio.Composition/RuntimeComposition.cs +++ b/src/Microsoft.VisualStudio.Composition/RuntimeComposition.cs @@ -321,13 +321,8 @@ public bool Equals(RuntimePart? other) [DebuggerDisplay("{" + nameof(ImportingSiteElementType) + "}")] public class RuntimeImport : IEquatable { - private NullableBool isLazy; - private Type? importingSiteElementType; - private Func, object, object>? lazyFactory; private ParameterInfo? importingParameter; private MemberInfo? importingMember; - private volatile bool isMetadataTypeInitialized; - private Type? metadataType; private RuntimeImport(TypeRef importingSiteTypeRef, TypeRef importingSiteTypeWithoutCollectionRef, ImportCardinality cardinality, IReadOnlyList satisfyingExports, bool isNonSharedInstanceRequired, bool isExportFactory, IReadOnlyDictionary metadata, IReadOnlyCollection exportFactorySharingBoundaries) { @@ -399,18 +394,7 @@ public Type? ExportFactory public ParameterInfo? ImportingParameter => this.importingParameter ?? (this.importingParameter = this.ImportingParameterRef?.ParameterInfo); - public bool IsLazy - { - get - { - if (!this.isLazy.HasValue) - { - this.isLazy = this.ImportingSiteTypeWithoutCollectionRef.IsAnyLazyType(); - } - - return this.isLazy.Value; - } - } + public bool IsLazy => this.ImportingSiteTypeWithoutCollectionRef.IsAnyLazyType(); public Type ImportingSiteType => this.ImportingSiteTypeRef.ResolvedType; @@ -421,32 +405,15 @@ public bool IsLazy /// /// Gets the type of the member, with the ImportMany collection and Lazy/ExportFactory stripped off, when present. /// - public Type ImportingSiteElementType - { - get - { - if (this.importingSiteElementType == null) - { - this.importingSiteElementType = PartDiscovery.GetTypeIdentityFromImportingType(this.ImportingSiteType, this.Cardinality == ImportCardinality.ZeroOrMore); - } - - return this.importingSiteElementType; - } - } + public Type ImportingSiteElementType => PartDiscovery.GetTypeIdentityFromImportingType(this.ImportingSiteType, this.Cardinality == ImportCardinality.ZeroOrMore); public Type? MetadataType { get { - if (!this.isMetadataTypeInitialized) - { - this.metadataType = this.IsLazy && this.ImportingSiteTypeWithoutCollection.GenericTypeArguments.Length == 2 - ? this.ImportingSiteTypeWithoutCollection.GenericTypeArguments[1] - : null; - this.isMetadataTypeInitialized = true; - } - - return this.metadataType; + return this.IsLazy && this.ImportingSiteTypeWithoutCollection.GenericTypeArguments.Length == 2 + ? this.ImportingSiteTypeWithoutCollection.GenericTypeArguments[1] + : null; } } @@ -462,13 +429,13 @@ public TypeRef DeclaringTypeRef { get { - if (this.lazyFactory == null && this.IsLazy) + if (!this.IsLazy) { - Type[] lazyTypeArgs = this.ImportingSiteTypeWithoutCollection.GenericTypeArguments; - this.lazyFactory = LazyServices.CreateStronglyTypedLazyFactory(this.ImportingSiteElementType, lazyTypeArgs.Length > 1 ? lazyTypeArgs[1] : null); + return null; } - return this.lazyFactory; + Type[] lazyTypeArgs = this.ImportingSiteTypeWithoutCollection.GenericTypeArguments; + return LazyServices.CreateStronglyTypedLazyFactory(this.ImportingSiteElementType, lazyTypeArgs.Length > 1 ? lazyTypeArgs[1] : null); } } From 0e0c2e1cb027df254384518a7be77b8632101c0c Mon Sep 17 00:00:00 2001 From: David Kean Date: Thu, 9 Apr 2026 13:52:33 +1000 Subject: [PATCH 4/5] Cache boxed CreationPolicy and common Int32 values in ReadObject Avoid per-call boxing for CreationPolicy enum (3 values) and small integers (0, 1, -1) that appear frequently in metadata. --- .../Configuration/SerializationContextBase.cs | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs b/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs index 81dccd950..93400ce1d 100644 --- a/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs +++ b/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs @@ -57,6 +57,12 @@ internal abstract partial class SerializationContextBase : IDisposable private static readonly object BoxedTrue = true; private static readonly object BoxedFalse = false; + private static readonly object BoxedCreationPolicyAny = CreationPolicy.Any; + private static readonly object BoxedCreationPolicyShared = CreationPolicy.Shared; + private static readonly object BoxedCreationPolicyNonShared = CreationPolicy.NonShared; + private static readonly object BoxedInt32Zero = 0; + private static readonly object BoxedInt32One = 1; + private static readonly object BoxedInt32NegativeOne = -1; private static readonly IReadOnlyDictionary EmptyMetadata = ImmutableDictionary.Empty; @@ -1084,7 +1090,14 @@ protected void WriteObject(object? value) case ObjectType.UInt64: return this.reader.ReadUInt64(); case ObjectType.Int32: - return this.reader.ReadInt32(); + int int32Value = this.reader.ReadInt32(); + return int32Value switch + { + 0 => BoxedInt32Zero, + 1 => BoxedInt32One, + -1 => BoxedInt32NegativeOne, + _ => int32Value, + }; case ObjectType.UInt32: return this.reader.ReadUInt32(); case ObjectType.Int16: @@ -1106,7 +1119,13 @@ protected void WriteObject(object? value) case ObjectType.Guid: return this.ReadGuid(); case ObjectType.CreationPolicy: - return (CreationPolicy)this.reader.ReadByte(); + return this.reader.ReadByte() switch + { + (byte)CreationPolicy.Any => BoxedCreationPolicyAny, + (byte)CreationPolicy.Shared => BoxedCreationPolicyShared, + (byte)CreationPolicy.NonShared => BoxedCreationPolicyNonShared, + byte b => (CreationPolicy)b, + }; case ObjectType.Type: return this.ReadTypeRef().Resolve(); case ObjectType.TypeRef: From d44ac2bb3872eb7fea555127844eb8bbce5e96c7 Mon Sep 17 00:00:00 2001 From: David Kean Date: Thu, 9 Apr 2026 13:52:48 +1000 Subject: [PATCH 5/5] Add typed fast paths in ReadArray for common element types Avoid Array.CreateInstance + Array.SetValue reflection overhead for object[], string[], and Type[] arrays in metadata deserialization. --- .../Configuration/SerializationContextBase.cs | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs b/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs index 93400ce1d..88a2679c6 100644 --- a/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs +++ b/src/Microsoft.VisualStudio.Composition/Configuration/SerializationContextBase.cs @@ -57,6 +57,8 @@ internal abstract partial class SerializationContextBase : IDisposable private static readonly object BoxedTrue = true; private static readonly object BoxedFalse = false; + + private static readonly IReadOnlyDictionary EmptyMetadata = ImmutableDictionary.Empty; private static readonly object BoxedCreationPolicyAny = CreationPolicy.Any; private static readonly object BoxedCreationPolicyShared = CreationPolicy.Shared; private static readonly object BoxedCreationPolicyNonShared = CreationPolicy.NonShared; @@ -64,8 +66,6 @@ internal abstract partial class SerializationContextBase : IDisposable private static readonly object BoxedInt32One = 1; private static readonly object BoxedInt32NegativeOne = -1; - private static readonly IReadOnlyDictionary EmptyMetadata = ImmutableDictionary.Empty; - internal SerializationContextBase(BinaryReader reader, Resolver resolver) { Requires.NotNull(reader, nameof(reader)); @@ -758,6 +758,41 @@ protected Array ReadArray(BinaryReader reader, Func itemReader, Type el throw new NotSupportedException(); } + // Use typed fast paths for common element types to avoid + // the reflection overhead of Array.CreateInstance + Array.SetValue. + if (elementType == typeof(object)) + { + var array = new object?[(int)count]; + for (int i = 0; i < array.Length; i++) + { + array[i] = itemReader(); + } + + return array; + } + + if (elementType == typeof(string)) + { + var array = new string?[(int)count]; + for (int i = 0; i < array.Length; i++) + { + array[i] = (string?)itemReader(); + } + + return array; + } + + if (elementType == typeof(Type)) + { + var array = new Type?[(int)count]; + for (int i = 0; i < array.Length; i++) + { + array[i] = (Type?)itemReader(); + } + + return array; + } + var list = Array.CreateInstance(elementType, (int)count); for (int i = 0; i < list.Length; i++) {