Reduce deserialization allocations by davkean · Pull Request #698 · microsoft/vs-mef

davkean · 2026-04-08T05:15:23Z

No description provided.

davkean · 2026-04-08T05:17:23Z

            this.resolver = resolver;
        }

+        internal LazyMetadataWrapper(ImmutableDictionary<string, object?> metadata, Direction direction, Resolver resolver)


Why is this constructor still needed?

davkean · 2026-04-08T05:22:52Z

-
-            // Update our metadata dictionary with the substitution to avoid
-            // the translation costs next time.
-            this.underlyingMetadata = this.underlyingMetadata.SetItem(key, value);


Why is this no longer applicable?

Who are you asking? I see a code comment below that seems to justify it.

It added this comment after I asked this. :) I could not get it to resolve the question.

davkean · 2026-04-08T06:29:34Z

-                serializedMetadata = new LazyMetadataWrapper(metadata.ToImmutableDictionary(), LazyMetadataWrapper.Direction.ToSubstitutedValue, this.Resolver);
+                // Both Dictionary and ImmutableDictionary implement IDictionary,
+                // which Dictionary's copy constructor accepts.
+                var metadataCopy = metadata is IDictionary<string, object?> dict


Is this guaranateed to be Dictionary<string, object?>? This also does not copy the metadata on the second path.

AArnott · 2026-04-08T17:16:43Z

        private static readonly object BoxedTrue = true;
        private static readonly object BoxedFalse = false;

+        private static readonly IReadOnlyDictionary<string, object?> EmptyMetadata = new Dictionary<string, object?>();


Make absolutely sure it's read-only and empty, and even avoid allocating the dictionary.

Suggested change

private static readonly IReadOnlyDictionary<string, object?> EmptyMetadata = new Dictionary<string, object?>();

private static readonly IReadOnlyDictionary<string, object?> EmptyMetadata = ImmutableDictionary<string, object?>.Empty;

AArnott · 2026-04-08T17:27:30Z

+                // Dictionary has no constructor accepting IReadOnlyDictionary, so enumerate manually.
+                var metadataCopy = new Dictionary<string, object?>(metadata.Count);
+                foreach (var entry in metadata)
+                {
+                    metadataCopy.Add(entry.Key, entry.Value);
+                }


No constructor, perhaps. But you can use this extension method, can't you?

Suggested change

// Dictionary has no constructor accepting IReadOnlyDictionary, so enumerate manually.

var metadataCopy = new Dictionary<string, object?>(metadata.Count);

foreach (var entry in metadata)

{

metadataCopy.Add(entry.Key, entry.Value);

}

Dictionary<string, object?> metadataCopy = metadata.ToDictionary();

AArnott · 2026-04-08T17:31:47Z

+            // Dictionary has no constructor accepting IReadOnlyDictionary, so enumerate manually.
+            var dict = new Dictionary<string, object?>(newMetadata.Count);
+            foreach (var kvp in newMetadata)
+            {
+                dict.Add(kvp.Key, kvp.Value);
+            }
+
+            return new LazyMetadataWrapper(dict, oldVersion.direction, this.resolver);


Suggested change

// Dictionary has no constructor accepting IReadOnlyDictionary, so enumerate manually.

var dict = new Dictionary<string, object?>(newMetadata.Count);

foreach (var kvp in newMetadata)

{

dict.Add(kvp.Key, kvp.Value);

}

return new LazyMetadataWrapper(dict, oldVersion.direction, this.resolver);

return new LazyMetadataWrapper(newMetadata.ToDictionary(), oldVersion.direction, this.resolver);

AArnott · 2026-04-08T17:33:22Z

-
-            // Update our metadata dictionary with the substitution to avoid
-            // the translation costs next time.
-            this.underlyingMetadata = this.underlyingMetadata.SetItem(key, value);


Who are you asking? I see a code comment below that seems to justify it.

AArnott · 2026-04-08T17:35:42Z

+            /// These involve CLR metadata resolution or reflection emit, so they are worth caching.
+            /// Allocated on first access to avoid paying the cost during composition cache loading.
+            /// </summary>
+            private sealed class ResolvedMembers


Wouldn't a nullable struct be even better?

AArnott · 2026-04-08T17:39:37Z

+            /// <summary>
+            /// Caches for resolved reflection objects and the lazy factory delegate.
+            /// These involve CLR metadata resolution or reflection emit, so they are worth caching.
+            /// Allocated on first access to avoid paying the cost during composition cache loading.
+            /// </summary>


We were already caching these. Why create a nested type? Was it to shrink the size of RuntimeImport objects? Particularly ones that never get accessed?

Yes to reduce the size of RuntimeImport so we only allocate when the data is accessed.

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.

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.

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<T> or Lazy<T, TMetadata> signature, eliminating per-import MakeGenericMethod and CreateDelegate calls.

Avoid per-call boxing for CreationPolicy enum (3 values) and small integers (0, 1, -1) that appear frequently in metadata.

Avoid Array.CreateInstance + Array.SetValue reflection overhead for object[], string[], and Type[] arrays in metadata deserialization.