From 9c179be804a5a7c5a70c4c6c9ce2b105d04ec78d Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Thu, 20 Aug 2026 22:57:32 -0600 Subject: [PATCH 1/4] Optimize export activation throughput Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ExportProvider.cs | 68 +++ .../ReflectionHelpers.cs | 65 +++ ...rtProviderFactory+RuntimeExportProvider.cs | 399 +++++++++++++++++- 3 files changed, 527 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.VisualStudio.Composition/ExportProvider.cs b/src/Microsoft.VisualStudio.Composition/ExportProvider.cs index 2063a2b6e..77c803acb 100644 --- a/src/Microsoft.VisualStudio.Composition/ExportProvider.cs +++ b/src/Microsoft.VisualStudio.Composition/ExportProvider.cs @@ -265,11 +265,21 @@ public Lazy GetExport(string? contractName) public T GetExportedValue() { + if (this.TryGetExportedValue(typeof(T), contractName: null, out object? value)) + { + return CastValueTo(value)!; + } + return this.GetExport().Value; } public T GetExportedValue(string? contractName) { + if (this.TryGetExportedValue(typeof(T), contractName, out object? value)) + { + return CastValueTo(value)!; + } + return this.GetExport(contractName).Value; } @@ -546,6 +556,19 @@ private protected static bool IsFullyInitializedExportRequiredWhenSettingImport( /// private protected abstract IEnumerable GetExportsCore(ImportDefinition importDefinition); + /// + /// Attempts to retrieve an exported value through a provider-specific optimized path. + /// + /// The exported value type. + /// The optional contract name. + /// Receives the exported value when the optimized path is available. + /// when was produced; otherwise, . + private protected virtual bool TryGetExportedValue(Type type, string? contractName, out object? value) + { + value = null; + return false; + } + private protected ExportInfo CreateExport(ImportDefinition importDefinition, IReadOnlyDictionary exportMetadata, TypeRef originalPartTypeRef, TypeRef constructedPartTypeRef, string? partSharingBoundary, bool nonSharedInstanceRequired, MemberRef? exportingMemberRef) { Requires.NotNull(importDefinition, nameof(importDefinition)); @@ -1215,6 +1238,11 @@ public object? Value /// protected abstract Type PartType { get; } + /// + /// Gets a value indicating whether this non-shared part has no lifecycle work beyond construction. + /// + protected virtual bool CanInitializeNonSharedValueDirectly => false; + /// /// Gets the instance of the part after fully initializing it. /// @@ -1226,6 +1254,11 @@ public object? Value /// public object? GetValueReadyToExpose() { + if (this.IsNonShared && this.State == PartLifecycleState.NotCreated && this.CanInitializeNonSharedValueDirectly) + { + return this.InitializeNonSharedValueDirectly(); + } + // If this very thread is already executing a step on this part, then we have some // form of reentrancy going on. In which case, the general policy seems to be that // we return an incompletely initialized part. @@ -1244,6 +1277,41 @@ public object? Value return this.Value; } + private object? InitializeNonSharedValueDirectly() + { + try + { + this.executingStepThreadId = Environment.CurrentManagedThreadId; + object? value = this.CreateValue(); + this.Value = value; + + if (value is IDisposable) + { + if (this.nonSharedPartOwner is null) + { + this.OwningExportProvider.TrackDisposableValue(this, sharingBoundary: null); + } + else + { + this.nonSharedPartOwner.AddNonSharedDescendant(this); + } + } + + Assumes.True(this.UpdateState(PartLifecycleState.Final)); + if (value is null) + { + this.ThrowPartNotInstantiableException(); + } + + return value; + } + catch (Exception ex) + { + this.Fault(ex); + throw; + } + } + /// /// Gets the instance of the part after instantiating it. /// Importing properties may not have been satisfied yet. diff --git a/src/Microsoft.VisualStudio.Composition/ReflectionHelpers.cs b/src/Microsoft.VisualStudio.Composition/ReflectionHelpers.cs index 2a44bd542..5e91ab3bb 100644 --- a/src/Microsoft.VisualStudio.Composition/ReflectionHelpers.cs +++ b/src/Microsoft.VisualStudio.Composition/ReflectionHelpers.cs @@ -701,6 +701,71 @@ internal static object Instantiate(this MethodBase ctorOrFactoryMethod, object?[ } } + /// + /// Creates a delegate that invokes a constructor or static factory method. + /// + /// The constructor or static factory method to invoke. + /// A delegate that accepts the invocation arguments and returns the constructed value. + internal static Func CreateInstanceFactory(this MethodBase ctorOrFactoryMethod) + { + Requires.NotNull(ctorOrFactoryMethod, nameof(ctorOrFactoryMethod)); + + var arguments = Expression.Parameter(typeof(object[]), "arguments"); + ParameterInfo[] parameters = ctorOrFactoryMethod.GetParameters(); + var convertedArguments = new Expression[parameters.Length]; + for (int i = 0; i < parameters.Length; i++) + { + Expression argument = Expression.ArrayIndex(arguments, Expression.Constant(i)); + convertedArguments[i] = ConvertInvocationValue(argument, parameters[i].ParameterType); + } + + Expression invocation = ctorOrFactoryMethod switch + { + ConstructorInfo constructor => Expression.New(constructor, convertedArguments), + MethodInfo method when method.IsStatic => Expression.Call(method, convertedArguments), + _ => throw ThrowUnsupportedImportingConstructor(ctorOrFactoryMethod), + }; + + return Expression.Lambda>( + Expression.Convert(invocation, typeof(object)), + arguments).Compile(); + } + + /// + /// Creates a delegate that assigns a value to an importing field or property. + /// + /// The importing field or property. + /// A delegate that assigns an imported value to a part instance. + internal static Action CreateImportingMemberSetter(this MemberInfo member) + { + Requires.NotNull(member, nameof(member)); + + var instance = Expression.Parameter(typeof(object), "instance"); + var value = Expression.Parameter(typeof(object), "value"); + Expression assignment = member switch + { + PropertyInfo property => Expression.Assign( + Expression.Property(Expression.Convert(instance, property.DeclaringType!), property), + ConvertInvocationValue(value, property.PropertyType)), + FieldInfo field => Expression.Assign( + Expression.Field(Expression.Convert(instance, field.DeclaringType!), field), + ConvertInvocationValue(value, field.FieldType)), + _ => throw new NotSupportedException(), + }; + + return Expression.Lambda>(assignment, instance, value).Compile(); + } + + private static Expression ConvertInvocationValue(Expression value, Type destinationType) + { + return destinationType.GetTypeInfo().IsValueType && Nullable.GetUnderlyingType(destinationType) is null + ? Expression.Condition( + Expression.ReferenceEqual(value, Expression.Constant(null)), + Expression.Default(destinationType), + Expression.Convert(value, destinationType)) + : Expression.Convert(value, destinationType); + } + [DoesNotReturn] internal static Exception ThrowUnsupportedImportingConstructor(MethodBase ctorOrFactoryMethod) { diff --git a/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs b/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs index 5a67f9464..a54df81bc 100644 --- a/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs +++ b/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs @@ -4,9 +4,11 @@ namespace Microsoft.VisualStudio.Composition { using System; + using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; + using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using System.Reflection; @@ -34,6 +36,9 @@ private class RuntimeExportProvider : ExportProvider private readonly RuntimeComposition composition; private readonly ReportFaultCallback? faultCallback; + private readonly ConcurrentDictionary> importingMemberSetterCache = new(); + private readonly ConcurrentDictionary> instanceFactoryCache = new(); + private readonly ConcurrentDictionary<(Type Type, string? ContractName), RuntimeExportLookup> runtimeExportLookupCache = new(); internal RuntimeExportProvider(RuntimeComposition composition, ReportFaultCallback faultCallback) : this(composition) @@ -72,6 +77,60 @@ select this.CreateExport( export.MemberRef); } + private protected override bool TryGetExportedValue(Type type, string? contractName, out object? value) + { + Verify.NotDisposed(this); + + contractName = string.IsNullOrEmpty(contractName) ? null : contractName; + if (!this.runtimeExportLookupCache.TryGetValue((type, contractName), out RuntimeExportLookup? lookup)) + { + lookup = this.CreateRuntimeExportLookup(type, contractName ?? ContractNameServices.GetTypeIdentity(type)); + lookup = this.runtimeExportLookupCache.GetOrAdd((type, contractName), lookup); + } + + Assumes.NotNull(lookup); + if (!lookup.CanUseFastPath) + { + value = null; + return false; + } + + if (lookup.TryGetSharedValue(out value)) + { + return true; + } + + value = this.GetRuntimeExportedValue(lookup, type); + lookup.SetSharedValue(value); + return true; + } + + private object? GetRuntimeExportedValue(RuntimeExportLookup lookup, Type type) + { + if (lookup.DirectValueFactory is object) + { + return lookup.DirectValueFactory(); + } + + MemberInfo? exportingMember = lookup.Export!.Member; + if (exportingMember?.IsStatic() == true) + { + return GetValueFromMember(null, exportingMember, type, lookup.Export.ExportedValueTypeRef.Resolve()); + } + + PartLifecycleTracker partLifecycle = this.GetOrCreateValue( + lookup.Part!.TypeRef, + lookup.Part.TypeRef, + lookup.Part.SharingBoundary, + EmptyMetadata, + !lookup.Part.IsShared, + nonSharedPartOwner: null); + object? part = partLifecycle.GetValueReadyToExpose(); + return lookup.Export.MemberRef is null + ? part + : GetValueFromMember(part, exportingMember!, type, lookup.Export.ExportedValueTypeRef.Resolve()); + } + internal override PartLifecycleTracker CreatePartLifecycleTracker(TypeRef partType, IReadOnlyDictionary importMetadata, PartLifecycleTracker? nonSharedPartOwner) { return nonSharedPartOwner is object @@ -79,6 +138,259 @@ internal override PartLifecycleTracker CreatePartLifecycleTracker(TypeRef partTy : new RuntimePartLifecycleTracker(this, this.composition.GetPart(partType), importMetadata); } + private RuntimeExportLookup CreateRuntimeExportLookup(Type type, string contractName) + { + IReadOnlyCollection exports = this.composition.GetExports(contractName); + string typeIdentity = ContractNameServices.GetTypeIdentity(type); + RuntimeComposition.RuntimeExport? matchingExport = null; + int matchingExportCount = 0; + foreach (RuntimeComposition.RuntimeExport export in exports) + { + if (export.Metadata.TryGetValue(CompositionConstants.ExportTypeIdentityMetadataName, out object? exportedTypeIdentity) + && string.Equals(typeIdentity, exportedTypeIdentity as string, StringComparison.Ordinal)) + { + matchingExport = export; + matchingExportCount++; + } + } + + if (matchingExportCount != 1) + { + return RuntimeExportLookup.Unsupported; + } + + RuntimeComposition.RuntimePart part = this.composition.GetPart(matchingExport!); + return part.TypeRef.IsGenericTypeDefinition + ? RuntimeExportLookup.Unsupported + : new RuntimeExportLookup(this, part, matchingExport!); + } + + private bool TryCreateDirectValueFactory( + RuntimeComposition.RuntimePart part, + RuntimeComposition.RuntimeExport export, + HashSet partsBeingBuilt, + [NotNullWhen(true)] out Func? valueFactory) + { + valueFactory = null; + if (part.IsShared + || export.MemberRef is object + || !part.IsInstantiable + || part.TypeRef.IsGenericTypeDefinition + || part.OnImportsSatisfiedMethodRefs.Count > 0 + || typeof(IDisposable).GetTypeInfo().IsAssignableFrom(part.TypeRef.Resolve().GetTypeInfo()) + || !partsBeingBuilt.Add(part.TypeRef)) + { + return false; + } + + try + { + MethodBase importingConstructorOrFactoryMethod = part.ImportingConstructorOrFactoryMethod!; + if (importingConstructorOrFactoryMethod is not ConstructorInfo + || importingConstructorOrFactoryMethod.ContainsGenericParameters) + { + return false; + } + + IReadOnlyList constructorImports = part.ImportingConstructorArguments; + var argumentFactories = new Func[constructorImports.Count]; + for (int i = 0; i < constructorImports.Count; i++) + { + RuntimeComposition.RuntimeImport import = constructorImports[i]; + if (import.IsLazy + || import.IsExportFactory + || import.IsNonSharedInstanceRequired) + { + return false; + } + + if (import.Cardinality == ImportCardinality.ZeroOrMore) + { + Type importingSiteType = import.ImportingSiteType; + if (!importingSiteType.IsArray + && (!importingSiteType.GetTypeInfo().IsGenericType + || !importingSiteType.GetGenericTypeDefinition().IsEquivalentTo(typeof(IEnumerable<>)))) + { + return false; + } + + Func[] elementFactories = new Func[import.SatisfyingExports.Count]; + int elementIndex = 0; + foreach (RuntimeComposition.RuntimeExport importedExport in import.SatisfyingExports) + { + if (!this.TryCreateDirectImportFactory(import, importedExport, partsBeingBuilt, out Func? elementFactory)) + { + return false; + } + + elementFactories[elementIndex++] = elementFactory; + } + + Type elementType = import.ImportingSiteTypeWithoutCollection; + argumentFactories[i] = () => + { + Array values = Array.CreateInstance(elementType, elementFactories.Length); + for (int j = 0; j < elementFactories.Length; j++) + { + values.SetValue(elementFactories[j](), j); + } + + return values; + }; + } + else + { + if (import.SatisfyingExports.Count != 1 + || !this.TryCreateDirectImportFactory(import, import.SatisfyingExports.First(), partsBeingBuilt, out Func? argumentFactory)) + { + return false; + } + + argumentFactories[i] = argumentFactory; + } + } + + IReadOnlyList memberImports = part.ImportingMembers; + var memberAssignments = new DirectMemberImport[memberImports.Count]; + for (int i = 0; i < memberImports.Count; i++) + { + RuntimeComposition.RuntimeImport import = memberImports[i]; + if (import.Cardinality != ImportCardinality.ExactlyOne + || import.IsLazy + || import.IsExportFactory + || import.IsNonSharedInstanceRequired + || import.SatisfyingExports.Count != 1 + || !this.TryCreateDirectImportFactory(import, import.SatisfyingExports.First(), partsBeingBuilt, out Func? importValueFactory)) + { + return false; + } + + memberAssignments[i] = new DirectMemberImport( + import, + importValueFactory, + this.GetOrCreateImportingMemberSetter(import.ImportingMember!)); + } + + Func instanceFactory = this.GetOrCreateInstanceFactory(importingConstructorOrFactoryMethod); + valueFactory = () => + { + object?[] arguments = argumentFactories.Length == 0 ? EmptyObjectArray : new object?[argumentFactories.Length]; + for (int i = 0; i < argumentFactories.Length; i++) + { + arguments[i] = argumentFactories[i](); + } + + object instance; + try + { + instance = instanceFactory(arguments)!; + Assumes.NotNull(instance); + } + catch (Exception ex) + { + throw new CompositionFailedException( + Strings.FormatExceptionThrownByPartUnderInitialization(part.TypeRef.Resolve().FullName), + ex); + } + + for (int i = 0; i < memberAssignments.Length; i++) + { + DirectMemberImport assignment = memberAssignments[i]; + object? importedValue; + try + { + importedValue = assignment.ValueFactory(); + } + catch (CompositionFailedException ex) + { + throw new CompositionFailedException( + string.Format( + CultureInfo.CurrentCulture, + Strings.ErrorWhileSettingImport, + RuntimeComposition.GetDiagnosticLocation(assignment.Import)), + ex); + } + + try + { + assignment.Setter(instance, importedValue); + } + catch (Exception ex) + { + throw new CompositionFailedException( + Strings.FormatExceptionThrownByPartUnderInitialization(part.TypeRef.Resolve().FullName), + ex); + } + } + + return instance; + }; + return true; + } + finally + { + partsBeingBuilt.Remove(part.TypeRef); + } + } + + private bool TryCreateDirectImportFactory( + RuntimeComposition.RuntimeImport import, + RuntimeComposition.RuntimeExport importedExport, + HashSet partsBeingBuilt, + [NotNullWhen(true)] out Func? valueFactory) + { + valueFactory = null; + if (importedExport.MemberRef is object) + { + return false; + } + + RuntimeComposition.RuntimePart importedPart = this.composition.GetPart(importedExport); + if (importedPart.TypeRef.IsGenericTypeDefinition) + { + return false; + } + + if (importedPart.IsShared) + { + var sharedLookup = new RuntimeExportLookup(this, importedPart, importedExport); + valueFactory = () => + { + if (sharedLookup.TryGetSharedValue(out object? sharedValue)) + { + return sharedValue; + } + + sharedValue = this.GetRuntimeExportedValue(sharedLookup, import.ImportingSiteTypeWithoutCollection); + sharedLookup.SetSharedValue(sharedValue); + return sharedValue; + }; + return true; + } + + return this.TryCreateDirectValueFactory(importedPart, importedExport, partsBeingBuilt, out valueFactory); + } + + private Action GetOrCreateImportingMemberSetter(MemberInfo member) + { + if (!this.importingMemberSetterCache.TryGetValue(member, out Action? setter)) + { + setter = this.importingMemberSetterCache.GetOrAdd(member, static member => member.CreateImportingMemberSetter()); + } + + return setter; + } + + private Func GetOrCreateInstanceFactory(MethodBase method) + { + if (!this.instanceFactoryCache.TryGetValue(method, out Func? factory)) + { + factory = this.instanceFactoryCache.GetOrAdd(method, static method => method.CreateInstanceFactory()); + } + + return factory; + } + internal override IMetadataViewProvider GetMetadataViewProvider(Type metadataView) { RuntimeComposition.RuntimeExport? metadataViewProviderExport; @@ -478,6 +790,70 @@ internal ValueForImportSite(object? value) public object? Value { get; private set; } } + private readonly struct DirectMemberImport + { + internal DirectMemberImport(RuntimeComposition.RuntimeImport import, Func valueFactory, Action setter) + { + this.Import = import; + this.ValueFactory = valueFactory; + this.Setter = setter; + } + + internal RuntimeComposition.RuntimeImport Import { get; } + + internal Action Setter { get; } + + internal Func ValueFactory { get; } + } + + private sealed class RuntimeExportLookup + { + internal static readonly RuntimeExportLookup Unsupported = new RuntimeExportLookup(); + private volatile bool hasSharedValue; + private object? sharedValue; + + private RuntimeExportLookup() + { + } + + internal RuntimeExportLookup(RuntimeExportProvider exportProvider, RuntimeComposition.RuntimePart part, RuntimeComposition.RuntimeExport export) + { + this.Part = part; + this.Export = export; + exportProvider.TryCreateDirectValueFactory(part, export, new HashSet(), out Func? directValueFactory); + this.DirectValueFactory = directValueFactory; + } + + internal bool CanUseFastPath => this.Part is object; + + internal RuntimeComposition.RuntimePart? Part { get; } + + internal RuntimeComposition.RuntimeExport? Export { get; } + + internal Func? DirectValueFactory { get; } + + internal bool TryGetSharedValue(out object? value) + { + if (this.hasSharedValue) + { + value = this.sharedValue; + return true; + } + + value = null; + return false; + } + + internal void SetSharedValue(object? value) + { + if (this.Part!.IsShared && this.Export!.MemberRef is null) + { + this.sharedValue = value; + this.hasSharedValue = true; + } + } + } + [DebuggerDisplay("{" + nameof(partDefinition) + "." + nameof(RuntimeComposition.RuntimePart.TypeRef) + "." + nameof(TypeRef.ResolvedType) + ".FullName,nq} ({State})")] private class RuntimePartLifecycleTracker : PartLifecycleTracker { @@ -519,6 +895,12 @@ protected override Type PartType get { return this.partDefinition.TypeRef.Resolve(); } } + protected override bool CanInitializeNonSharedValueDirectly => + this.partDefinition.IsInstantiable + && this.partDefinition.ImportingConstructorArguments.Count == 0 + && this.partDefinition.ImportingMembers.Count == 0 + && this.partDefinition.OnImportsSatisfiedMethodRefs.Count == 0; + internal new void ReportPartiallyInitializedImport(PartLifecycleTracker part) { base.ReportPartiallyInitializedImport(part); @@ -538,8 +920,13 @@ protected override Type PartType } var constructedPartTypeRef = GetPartConstructedTypeRef(this.partDefinition, this.importMetadata); - var ctorArgs = this.partDefinition.ImportingConstructorArguments - .Select(import => this.OwningExportProvider.GetValueForImportSite(this, import).Value).ToArray(); + IReadOnlyList constructorImports = this.partDefinition.ImportingConstructorArguments; + object?[] ctorArgs = constructorImports.Count == 0 ? EmptyObjectArray : new object?[constructorImports.Count]; + for (int i = 0; i < constructorImports.Count; i++) + { + ctorArgs[i] = this.OwningExportProvider.GetValueForImportSite(this, constructorImports[i]).Value; + } + MethodBase? importingConstructorOrFactoryMethod = this.partDefinition.ImportingConstructorOrFactoryMethod!; if (importingConstructorOrFactoryMethod.ContainsGenericParameters) { @@ -551,12 +938,14 @@ protected override Type PartType try { - object? part = importingConstructorOrFactoryMethod.Instantiate(ctorArgs); + object? part = this.IsNonShared + ? this.OwningExportProvider.GetOrCreateInstanceFactory(importingConstructorOrFactoryMethod)(ctorArgs) + : importingConstructorOrFactoryMethod.Instantiate(ctorArgs); return part; } - catch (TargetInvocationException ex) + catch (Exception ex) { - throw this.PrepareExceptionForFaultedPart(ex); + throw this.PrepareExceptionForFaultedPart(ex as TargetInvocationException ?? new TargetInvocationException(ex)); } } From f795232bb5dcec549fc9e9c81446cd1c76fe15c3 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 21 Aug 2026 09:49:53 -0600 Subject: [PATCH 2/4] Add activation performance benchmarks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ActivationBenchmarks.cs | 331 ++++++++++++++++++ 1 file changed, 331 insertions(+) create mode 100644 test/Microsoft.VisualStudio.Composition.Benchmarks/ActivationBenchmarks.cs diff --git a/test/Microsoft.VisualStudio.Composition.Benchmarks/ActivationBenchmarks.cs b/test/Microsoft.VisualStudio.Composition.Benchmarks/ActivationBenchmarks.cs new file mode 100644 index 000000000..e17ad10d2 --- /dev/null +++ b/test/Microsoft.VisualStudio.Composition.Benchmarks/ActivationBenchmarks.cs @@ -0,0 +1,331 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.VisualStudio.Composition.Benchmarks; + +using System.Composition; +using BenchmarkDotNet.Attributes; +using Microsoft.VSDiagnostics; + +/// +/// Measures steady-state activation of representative MEF part graphs. +/// +[MemoryDiagnoser] +[CPUUsageDiagnoser] +public class ActivationBenchmarks +{ + private ExportProvider exportProvider = null!; + + /// + /// Creates and warms the export provider used by the activation benchmarks. + /// + [GlobalSetup] + public void Setup() + { + var resolver = Resolver.DefaultInstance; + var discovery = new AttributedPartDiscovery(resolver, isNonPublicSupported: true); + var discoveredParts = discovery.CreatePartsAsync( + typeof(SingletonPart), + typeof(TransientPart), + typeof(CombinedPart), + typeof(SharedDependency), + typeof(TransientDependency), + typeof(ComplexPart), + typeof(ComplexServiceA), + typeof(ComplexServiceB), + typeof(ComplexServiceC), + typeof(ComplexSubObjectA), + typeof(ComplexSubObjectB), + typeof(ComplexSubObjectC), + typeof(PropertyPart), + typeof(PropertyServiceA), + typeof(PropertyServiceB), + typeof(PropertyServiceC), + typeof(PropertySubObjectA), + typeof(PropertySubObjectB), + typeof(PropertySubObjectC), + typeof(ImportManyPart), + typeof(AdapterA), + typeof(AdapterB), + typeof(AdapterC), + typeof(AdapterD), + typeof(AdapterE)).GetAwaiter().GetResult(); + + var catalog = ComposableCatalog.Create(resolver).AddParts(discoveredParts); + this.exportProvider = CompositionConfiguration.Create(catalog) + .CreateExportProviderFactory() + .CreateExportProvider(); + + this.Singleton(); + this.Transient(); + this.Combined(); + this.Complex(); + this.Property(); + this.ImportMany(); + } + + /// + /// Disposes the export provider after benchmarking. + /// + [GlobalCleanup] + public void Cleanup() => this.exportProvider.Dispose(); + + /// + /// Resolves an already initialized shared part. + /// + /// The last resolved part. + [Benchmark(OperationsPerInvoke = 3)] + public object Singleton() + { + _ = this.exportProvider.GetExportedValue(); + _ = this.exportProvider.GetExportedValue(); + return this.exportProvider.GetExportedValue(); + } + + /// + /// Resolves a non-shared part with no imports. + /// + /// The last resolved part. + [Benchmark(OperationsPerInvoke = 3)] + public object Transient() + { + _ = this.exportProvider.GetExportedValue(); + _ = this.exportProvider.GetExportedValue(); + return this.exportProvider.GetExportedValue(); + } + + /// + /// Resolves a non-shared part with shared and non-shared constructor imports. + /// + /// The last resolved part. + [Benchmark(OperationsPerInvoke = 3)] + public object Combined() + { + _ = this.exportProvider.GetExportedValue(); + _ = this.exportProvider.GetExportedValue(); + return this.exportProvider.GetExportedValue(); + } + + /// + /// Resolves an acyclic non-shared constructor-import graph. + /// + /// The last resolved part. + [Benchmark(OperationsPerInvoke = 3)] + public object Complex() + { + _ = this.exportProvider.GetExportedValue(); + _ = this.exportProvider.GetExportedValue(); + return this.exportProvider.GetExportedValue(); + } + + /// + /// Resolves a non-shared graph composed with property imports. + /// + /// The last resolved part. + [Benchmark(OperationsPerInvoke = 3)] + public object Property() + { + _ = this.exportProvider.GetExportedValue(); + _ = this.exportProvider.GetExportedValue(); + return this.exportProvider.GetExportedValue(); + } + + /// + /// Resolves a non-shared part with a constructor collection import. + /// + /// The last resolved part. + [Benchmark(OperationsPerInvoke = 3)] + public object ImportMany() + { + _ = this.exportProvider.GetExportedValue(); + _ = this.exportProvider.GetExportedValue(); + return this.exportProvider.GetExportedValue(); + } + + [Export] + [Shared] + private sealed class SingletonPart + { + } + + [Export] + private sealed class TransientPart + { + } + + [Export] + private sealed class CombinedPart + { + [ImportingConstructor] + internal CombinedPart(SharedDependency sharedDependency, TransientDependency transientDependency) + { + } + } + + [Export] + [Shared] + private sealed class SharedDependency + { + } + + [Export] + private sealed class TransientDependency + { + } + + [Export] + private sealed class ComplexPart + { + [ImportingConstructor] + internal ComplexPart( + ComplexServiceA serviceA, + ComplexServiceB serviceB, + ComplexServiceC serviceC, + ComplexSubObjectA subObjectA, + ComplexSubObjectB subObjectB, + ComplexSubObjectC subObjectC) + { + } + } + + [Export] + private sealed class ComplexServiceA + { + } + + [Export] + private sealed class ComplexServiceB + { + } + + [Export] + private sealed class ComplexServiceC + { + } + + [Export] + private sealed class ComplexSubObjectA + { + [ImportingConstructor] + internal ComplexSubObjectA(ComplexServiceA service) + { + } + } + + [Export] + private sealed class ComplexSubObjectB + { + [ImportingConstructor] + internal ComplexSubObjectB(ComplexServiceB service) + { + } + } + + [Export] + private sealed class ComplexSubObjectC + { + [ImportingConstructor] + internal ComplexSubObjectC(ComplexServiceC service) + { + } + } + + [Export] + private sealed class PropertyPart + { + [Import] + internal PropertyServiceA ServiceA { get; set; } = null!; + + [Import] + internal PropertyServiceB ServiceB { get; set; } = null!; + + [Import] + internal PropertyServiceC ServiceC { get; set; } = null!; + + [Import] + internal PropertySubObjectA SubObjectA { get; set; } = null!; + + [Import] + internal PropertySubObjectB SubObjectB { get; set; } = null!; + + [Import] + internal PropertySubObjectC SubObjectC { get; set; } = null!; + } + + [Export] + [Shared] + private sealed class PropertyServiceA + { + } + + [Export] + [Shared] + private sealed class PropertyServiceB + { + } + + [Export] + [Shared] + private sealed class PropertyServiceC + { + } + + [Export] + private sealed class PropertySubObjectA + { + [Import] + internal PropertyServiceA Service { get; set; } = null!; + } + + [Export] + private sealed class PropertySubObjectB + { + [Import] + internal PropertyServiceB Service { get; set; } = null!; + } + + [Export] + private sealed class PropertySubObjectC + { + [Import] + internal PropertyServiceC Service { get; set; } = null!; + } + + [Export] + private sealed class ImportManyPart + { + [ImportingConstructor] + internal ImportManyPart([ImportMany] IEnumerable adapters) + { + } + } + + private interface IAdapter + { + } + + [Export(typeof(IAdapter))] + private sealed class AdapterA : IAdapter + { + } + + [Export(typeof(IAdapter))] + private sealed class AdapterB : IAdapter + { + } + + [Export(typeof(IAdapter))] + private sealed class AdapterC : IAdapter + { + } + + [Export(typeof(IAdapter))] + private sealed class AdapterD : IAdapter + { + } + + [Export(typeof(IAdapter))] + private sealed class AdapterE : IAdapter + { + } +} From f77ce3ec876121737526c25ad241ec2bfc0f758a Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 24 Aug 2026 09:51:07 -0600 Subject: [PATCH 3/4] Separate compiled activation experiment Document the activation performance investigation and retain only the non-expression optimizations on the active PR branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PERFORMANCE.md | 102 +++++++ .../ReflectionHelpers.cs | 65 ----- ...rtProviderFactory+RuntimeExportProvider.cs | 266 +----------------- 3 files changed, 107 insertions(+), 326 deletions(-) create mode 100644 PERFORMANCE.md diff --git a/PERFORMANCE.md b/PERFORMANCE.md new file mode 100644 index 000000000..9c3c5859c --- /dev/null +++ b/PERFORMANCE.md @@ -0,0 +1,102 @@ +# Performance + +This document records performance investigations, implemented optimizations, and possible future work. + +## Activation benchmarks + +`test/Microsoft.VisualStudio.Composition.Benchmarks/ActivationBenchmarks.cs` covers six steady-state activation shapes: + +- Retrieval of a shared part. +- Activation of a non-shared part without imports. +- Activation with shared and non-shared constructor imports. +- Activation of a larger acyclic constructor-import graph. +- Activation of a property-import graph. +- Activation with an `ImportMany` constructor parameter. + +Run these benchmarks with: + +```powershell +dotnet run --project test\Microsoft.VisualStudio.Composition.Benchmarks\Microsoft.VisualStudio.Composition.Benchmarks.csproj -c Release --framework net10.0 -- --filter "*ActivationBenchmarks*" +``` + +Benchmark results should be considered together with startup time, allocation data, and the number of generated or JIT-compiled methods. + +## Low-JIT-cost optimizations + +The `perf-optimizations` branch and its pull request deliberately avoid adding expression-compiled activation methods. They include: + +- Caching exact runtime export lookups for `GetExportedValue()`. +- Caching initialized shared root values without using `Lazy`, which would break supported reentrant activation. +- Bypassing general lifecycle state transitions for non-shared parts that have no imports or `OnImportsSatisfied` callbacks. +- Avoiding LINQ and empty-array allocations while resolving constructor arguments through the normal lifecycle engine. + +These changes primarily improve shared export retrieval and simple non-shared activation. They preserve the existing reflection and lifecycle behavior for imported graphs. + +## Compiled activation experiment + +The `perf/compiled-activation-plans` branch preserves the expression-compilation work separately. It adds: + +- Compiled constructor delegates for repeatedly activated non-shared parts. +- Compiled property and field setters. +- Recursive direct activation plans for supported acyclic non-shared graphs. +- Direct construction of constructor imports, property imports, and array or `IEnumerable` imports. + +Unsupported cases fall back to the normal lifecycle engine. These include cycles, disposable parts, open generic parts, lazy imports, export factories, exported members, custom collections, and `OnImportsSatisfied`. + +The compiled approach substantially improves throughput and allocations, but it also creates many additional generated methods that must be JIT-compiled. A short BenchmarkDotNet run on one machine produced the following indicative results: + +| Scenario | Without compiled activation | With compiled activation | +| --- | ---: | ---: | +| Shared | 15.62 ns, 0 B | 17.57 ns, 0 B | +| Simple non-shared | 477.01 ns, 160 B | 30.86 ns, 24 B | +| Constructor imports | 1,732.26 ns, 712 B | 61.97 ns, 88 B | +| Complex constructor graph | 7,605.75 ns, 3,176 B | 246.24 ns, 408 B | +| Property imports | 5,661.30 ns, 2,440 B | 115.68 ns, 136 B | +| `ImportMany` | 3,772.01 ns, 1,440 B | 227.34 ns, 240 B | + +These numbers came from separate BenchmarkDotNet short runs and are intended to show the magnitude of the tradeoff, not to establish release-quality baselines. + +Compiling activation for an application-wide shared part is generally unattractive because it increases startup and JIT cost for an operation that normally runs only once. Compilation is more likely to pay for: + +- Non-shared parts that may be activated repeatedly. +- Parts shared within a sharing boundary that is instantiated repeatedly. +- Eager subgraphs rooted at either of those part categories. + +## Future activation work + +Future work should be incremental and should measure JIT and startup costs as first-class outcomes. + +### 1. Hybrid per-part plans + +The current experimental direct plan is all-or-nothing: one unsupported node rejects the complete eager activation closure. A hybrid plan could pre-resolve imports and optimize supported steps while delegating unsupported edges to the existing lifecycle engine. + +This would let ordinary constructor and property activation benefit even when a larger graph includes cycles, disposables, lazy imports, export factories, callbacks, or generic parts. Keeping lifecycle trackers at this stage should limit semantic risk. + +### 2. Selective compilation + +Expression compilation should be restricted to parts with a credible opportunity to amortize its startup and JIT cost. Likely candidates are: + +- Non-shared parts observed or expected to be activated repeatedly. +- Parts shared in a repeatedly created sharing boundary. + +Application-wide shared parts should normally remain on the reflection path. Selection may be based on composition metadata and sharing policy rather than compiling every eligible part eagerly. + +### 3. Fused eager subgraphs + +For a repeatedly activated root, build a typed delegate for the eager activation closure that must be created synchronously with that root. This is not the whole application graph. The closure stops at existing shared values, lazy imports, export factories, unsupported lifecycle edges, and other deferred boundaries. + +A fused delegate could use typed locals, direct constructor calls, direct member assignments, and typed collection creation. This can eliminate intermediate `object[]` arrays, recursive `Func` dispatch, repeated casts, and reflection-based collection assignment. + +Large plans should be segmented at sharing and lifecycle boundaries to avoid very large generated methods. Compilation may also need thresholds based on expected activation count or graph size. + +## Required measurements + +Any compiled or hybrid proposal should compare: + +- Provider creation and first-activation time. +- Steady-state activation throughput. +- Managed allocations. +- Number and size of generated methods. +- JIT time and generated native code size. +- Behavior on representative Visual Studio compositions and repeated sharing-boundary activation. +- Functional compatibility for cycles, reentrancy, disposal ownership, exceptions, generic closing, and `OnImportsSatisfied`. diff --git a/src/Microsoft.VisualStudio.Composition/ReflectionHelpers.cs b/src/Microsoft.VisualStudio.Composition/ReflectionHelpers.cs index 5e91ab3bb..2a44bd542 100644 --- a/src/Microsoft.VisualStudio.Composition/ReflectionHelpers.cs +++ b/src/Microsoft.VisualStudio.Composition/ReflectionHelpers.cs @@ -701,71 +701,6 @@ internal static object Instantiate(this MethodBase ctorOrFactoryMethod, object?[ } } - /// - /// Creates a delegate that invokes a constructor or static factory method. - /// - /// The constructor or static factory method to invoke. - /// A delegate that accepts the invocation arguments and returns the constructed value. - internal static Func CreateInstanceFactory(this MethodBase ctorOrFactoryMethod) - { - Requires.NotNull(ctorOrFactoryMethod, nameof(ctorOrFactoryMethod)); - - var arguments = Expression.Parameter(typeof(object[]), "arguments"); - ParameterInfo[] parameters = ctorOrFactoryMethod.GetParameters(); - var convertedArguments = new Expression[parameters.Length]; - for (int i = 0; i < parameters.Length; i++) - { - Expression argument = Expression.ArrayIndex(arguments, Expression.Constant(i)); - convertedArguments[i] = ConvertInvocationValue(argument, parameters[i].ParameterType); - } - - Expression invocation = ctorOrFactoryMethod switch - { - ConstructorInfo constructor => Expression.New(constructor, convertedArguments), - MethodInfo method when method.IsStatic => Expression.Call(method, convertedArguments), - _ => throw ThrowUnsupportedImportingConstructor(ctorOrFactoryMethod), - }; - - return Expression.Lambda>( - Expression.Convert(invocation, typeof(object)), - arguments).Compile(); - } - - /// - /// Creates a delegate that assigns a value to an importing field or property. - /// - /// The importing field or property. - /// A delegate that assigns an imported value to a part instance. - internal static Action CreateImportingMemberSetter(this MemberInfo member) - { - Requires.NotNull(member, nameof(member)); - - var instance = Expression.Parameter(typeof(object), "instance"); - var value = Expression.Parameter(typeof(object), "value"); - Expression assignment = member switch - { - PropertyInfo property => Expression.Assign( - Expression.Property(Expression.Convert(instance, property.DeclaringType!), property), - ConvertInvocationValue(value, property.PropertyType)), - FieldInfo field => Expression.Assign( - Expression.Field(Expression.Convert(instance, field.DeclaringType!), field), - ConvertInvocationValue(value, field.FieldType)), - _ => throw new NotSupportedException(), - }; - - return Expression.Lambda>(assignment, instance, value).Compile(); - } - - private static Expression ConvertInvocationValue(Expression value, Type destinationType) - { - return destinationType.GetTypeInfo().IsValueType && Nullable.GetUnderlyingType(destinationType) is null - ? Expression.Condition( - Expression.ReferenceEqual(value, Expression.Constant(null)), - Expression.Default(destinationType), - Expression.Convert(value, destinationType)) - : Expression.Convert(value, destinationType); - } - [DoesNotReturn] internal static Exception ThrowUnsupportedImportingConstructor(MethodBase ctorOrFactoryMethod) { diff --git a/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs b/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs index a54df81bc..df0267c4e 100644 --- a/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs +++ b/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs @@ -8,7 +8,6 @@ namespace Microsoft.VisualStudio.Composition using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; - using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using System.Reflection; @@ -36,8 +35,6 @@ private class RuntimeExportProvider : ExportProvider private readonly RuntimeComposition composition; private readonly ReportFaultCallback? faultCallback; - private readonly ConcurrentDictionary> importingMemberSetterCache = new(); - private readonly ConcurrentDictionary> instanceFactoryCache = new(); private readonly ConcurrentDictionary<(Type Type, string? ContractName), RuntimeExportLookup> runtimeExportLookupCache = new(); internal RuntimeExportProvider(RuntimeComposition composition, ReportFaultCallback faultCallback) @@ -107,11 +104,6 @@ private protected override bool TryGetExportedValue(Type type, string? contractN private object? GetRuntimeExportedValue(RuntimeExportLookup lookup, Type type) { - if (lookup.DirectValueFactory is object) - { - return lookup.DirectValueFactory(); - } - MemberInfo? exportingMember = lookup.Export!.Member; if (exportingMember?.IsStatic() == true) { @@ -162,233 +154,7 @@ private RuntimeExportLookup CreateRuntimeExportLookup(Type type, string contract RuntimeComposition.RuntimePart part = this.composition.GetPart(matchingExport!); return part.TypeRef.IsGenericTypeDefinition ? RuntimeExportLookup.Unsupported - : new RuntimeExportLookup(this, part, matchingExport!); - } - - private bool TryCreateDirectValueFactory( - RuntimeComposition.RuntimePart part, - RuntimeComposition.RuntimeExport export, - HashSet partsBeingBuilt, - [NotNullWhen(true)] out Func? valueFactory) - { - valueFactory = null; - if (part.IsShared - || export.MemberRef is object - || !part.IsInstantiable - || part.TypeRef.IsGenericTypeDefinition - || part.OnImportsSatisfiedMethodRefs.Count > 0 - || typeof(IDisposable).GetTypeInfo().IsAssignableFrom(part.TypeRef.Resolve().GetTypeInfo()) - || !partsBeingBuilt.Add(part.TypeRef)) - { - return false; - } - - try - { - MethodBase importingConstructorOrFactoryMethod = part.ImportingConstructorOrFactoryMethod!; - if (importingConstructorOrFactoryMethod is not ConstructorInfo - || importingConstructorOrFactoryMethod.ContainsGenericParameters) - { - return false; - } - - IReadOnlyList constructorImports = part.ImportingConstructorArguments; - var argumentFactories = new Func[constructorImports.Count]; - for (int i = 0; i < constructorImports.Count; i++) - { - RuntimeComposition.RuntimeImport import = constructorImports[i]; - if (import.IsLazy - || import.IsExportFactory - || import.IsNonSharedInstanceRequired) - { - return false; - } - - if (import.Cardinality == ImportCardinality.ZeroOrMore) - { - Type importingSiteType = import.ImportingSiteType; - if (!importingSiteType.IsArray - && (!importingSiteType.GetTypeInfo().IsGenericType - || !importingSiteType.GetGenericTypeDefinition().IsEquivalentTo(typeof(IEnumerable<>)))) - { - return false; - } - - Func[] elementFactories = new Func[import.SatisfyingExports.Count]; - int elementIndex = 0; - foreach (RuntimeComposition.RuntimeExport importedExport in import.SatisfyingExports) - { - if (!this.TryCreateDirectImportFactory(import, importedExport, partsBeingBuilt, out Func? elementFactory)) - { - return false; - } - - elementFactories[elementIndex++] = elementFactory; - } - - Type elementType = import.ImportingSiteTypeWithoutCollection; - argumentFactories[i] = () => - { - Array values = Array.CreateInstance(elementType, elementFactories.Length); - for (int j = 0; j < elementFactories.Length; j++) - { - values.SetValue(elementFactories[j](), j); - } - - return values; - }; - } - else - { - if (import.SatisfyingExports.Count != 1 - || !this.TryCreateDirectImportFactory(import, import.SatisfyingExports.First(), partsBeingBuilt, out Func? argumentFactory)) - { - return false; - } - - argumentFactories[i] = argumentFactory; - } - } - - IReadOnlyList memberImports = part.ImportingMembers; - var memberAssignments = new DirectMemberImport[memberImports.Count]; - for (int i = 0; i < memberImports.Count; i++) - { - RuntimeComposition.RuntimeImport import = memberImports[i]; - if (import.Cardinality != ImportCardinality.ExactlyOne - || import.IsLazy - || import.IsExportFactory - || import.IsNonSharedInstanceRequired - || import.SatisfyingExports.Count != 1 - || !this.TryCreateDirectImportFactory(import, import.SatisfyingExports.First(), partsBeingBuilt, out Func? importValueFactory)) - { - return false; - } - - memberAssignments[i] = new DirectMemberImport( - import, - importValueFactory, - this.GetOrCreateImportingMemberSetter(import.ImportingMember!)); - } - - Func instanceFactory = this.GetOrCreateInstanceFactory(importingConstructorOrFactoryMethod); - valueFactory = () => - { - object?[] arguments = argumentFactories.Length == 0 ? EmptyObjectArray : new object?[argumentFactories.Length]; - for (int i = 0; i < argumentFactories.Length; i++) - { - arguments[i] = argumentFactories[i](); - } - - object instance; - try - { - instance = instanceFactory(arguments)!; - Assumes.NotNull(instance); - } - catch (Exception ex) - { - throw new CompositionFailedException( - Strings.FormatExceptionThrownByPartUnderInitialization(part.TypeRef.Resolve().FullName), - ex); - } - - for (int i = 0; i < memberAssignments.Length; i++) - { - DirectMemberImport assignment = memberAssignments[i]; - object? importedValue; - try - { - importedValue = assignment.ValueFactory(); - } - catch (CompositionFailedException ex) - { - throw new CompositionFailedException( - string.Format( - CultureInfo.CurrentCulture, - Strings.ErrorWhileSettingImport, - RuntimeComposition.GetDiagnosticLocation(assignment.Import)), - ex); - } - - try - { - assignment.Setter(instance, importedValue); - } - catch (Exception ex) - { - throw new CompositionFailedException( - Strings.FormatExceptionThrownByPartUnderInitialization(part.TypeRef.Resolve().FullName), - ex); - } - } - - return instance; - }; - return true; - } - finally - { - partsBeingBuilt.Remove(part.TypeRef); - } - } - - private bool TryCreateDirectImportFactory( - RuntimeComposition.RuntimeImport import, - RuntimeComposition.RuntimeExport importedExport, - HashSet partsBeingBuilt, - [NotNullWhen(true)] out Func? valueFactory) - { - valueFactory = null; - if (importedExport.MemberRef is object) - { - return false; - } - - RuntimeComposition.RuntimePart importedPart = this.composition.GetPart(importedExport); - if (importedPart.TypeRef.IsGenericTypeDefinition) - { - return false; - } - - if (importedPart.IsShared) - { - var sharedLookup = new RuntimeExportLookup(this, importedPart, importedExport); - valueFactory = () => - { - if (sharedLookup.TryGetSharedValue(out object? sharedValue)) - { - return sharedValue; - } - - sharedValue = this.GetRuntimeExportedValue(sharedLookup, import.ImportingSiteTypeWithoutCollection); - sharedLookup.SetSharedValue(sharedValue); - return sharedValue; - }; - return true; - } - - return this.TryCreateDirectValueFactory(importedPart, importedExport, partsBeingBuilt, out valueFactory); - } - - private Action GetOrCreateImportingMemberSetter(MemberInfo member) - { - if (!this.importingMemberSetterCache.TryGetValue(member, out Action? setter)) - { - setter = this.importingMemberSetterCache.GetOrAdd(member, static member => member.CreateImportingMemberSetter()); - } - - return setter; - } - - private Func GetOrCreateInstanceFactory(MethodBase method) - { - if (!this.instanceFactoryCache.TryGetValue(method, out Func? factory)) - { - factory = this.instanceFactoryCache.GetOrAdd(method, static method => method.CreateInstanceFactory()); - } - - return factory; + : new RuntimeExportLookup(part, matchingExport!); } internal override IMetadataViewProvider GetMetadataViewProvider(Type metadataView) @@ -790,22 +556,6 @@ internal ValueForImportSite(object? value) public object? Value { get; private set; } } - private readonly struct DirectMemberImport - { - internal DirectMemberImport(RuntimeComposition.RuntimeImport import, Func valueFactory, Action setter) - { - this.Import = import; - this.ValueFactory = valueFactory; - this.Setter = setter; - } - - internal RuntimeComposition.RuntimeImport Import { get; } - - internal Action Setter { get; } - - internal Func ValueFactory { get; } - } - private sealed class RuntimeExportLookup { internal static readonly RuntimeExportLookup Unsupported = new RuntimeExportLookup(); @@ -816,12 +566,10 @@ private RuntimeExportLookup() { } - internal RuntimeExportLookup(RuntimeExportProvider exportProvider, RuntimeComposition.RuntimePart part, RuntimeComposition.RuntimeExport export) + internal RuntimeExportLookup(RuntimeComposition.RuntimePart part, RuntimeComposition.RuntimeExport export) { this.Part = part; this.Export = export; - exportProvider.TryCreateDirectValueFactory(part, export, new HashSet(), out Func? directValueFactory); - this.DirectValueFactory = directValueFactory; } internal bool CanUseFastPath => this.Part is object; @@ -830,8 +578,6 @@ internal RuntimeExportLookup(RuntimeExportProvider exportProvider, RuntimeCompos internal RuntimeComposition.RuntimeExport? Export { get; } - internal Func? DirectValueFactory { get; } - internal bool TryGetSharedValue(out object? value) { if (this.hasSharedValue) @@ -938,14 +684,12 @@ protected override Type PartType try { - object? part = this.IsNonShared - ? this.OwningExportProvider.GetOrCreateInstanceFactory(importingConstructorOrFactoryMethod)(ctorArgs) - : importingConstructorOrFactoryMethod.Instantiate(ctorArgs); + object? part = importingConstructorOrFactoryMethod.Instantiate(ctorArgs); return part; } - catch (Exception ex) + catch (TargetInvocationException ex) { - throw this.PrepareExceptionForFaultedPart(ex as TargetInvocationException ?? new TargetInvocationException(ex)); + throw this.PrepareExceptionForFaultedPart(ex); } } From 12887c5d8f9cbe3e3a4438c4f6b4e42aa3454220 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Thu, 27 Aug 2026 19:58:54 -0600 Subject: [PATCH 4/4] Address performance PR review feedback Avoid caching reentrant shared values before lifecycle finalization and make the performance documentation branch-independent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PERFORMANCE.md | 2 +- ...rtProviderFactory+RuntimeExportProvider.cs | 12 +++- .../ThreadSafetyTests.cs | 66 +++++++++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/PERFORMANCE.md b/PERFORMANCE.md index 9c3c5859c..b5a8a8d76 100644 --- a/PERFORMANCE.md +++ b/PERFORMANCE.md @@ -23,7 +23,7 @@ Benchmark results should be considered together with startup time, allocation da ## Low-JIT-cost optimizations -The `perf-optimizations` branch and its pull request deliberately avoid adding expression-compiled activation methods. They include: +The low-JIT-cost optimizations deliberately avoid adding expression-compiled activation methods. They include: - Caching exact runtime export lookups for `GetExportedValue()`. - Caching initialized shared root values without using `Lazy`, which would break supported reentrant activation. diff --git a/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs b/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs index df0267c4e..0d23898d7 100644 --- a/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs +++ b/src/Microsoft.VisualStudio.Composition/RuntimeExportProviderFactory+RuntimeExportProvider.cs @@ -97,13 +97,18 @@ private protected override bool TryGetExportedValue(Type type, string? contractN return true; } - value = this.GetRuntimeExportedValue(lookup, type); - lookup.SetSharedValue(value); + value = this.GetRuntimeExportedValue(lookup, type, out bool isFullyInitialized); + if (isFullyInitialized) + { + lookup.SetSharedValue(value); + } + return true; } - private object? GetRuntimeExportedValue(RuntimeExportLookup lookup, Type type) + private object? GetRuntimeExportedValue(RuntimeExportLookup lookup, Type type, out bool isFullyInitialized) { + isFullyInitialized = false; MemberInfo? exportingMember = lookup.Export!.Member; if (exportingMember?.IsStatic() == true) { @@ -118,6 +123,7 @@ private protected override bool TryGetExportedValue(Type type, string? contractN !lookup.Part.IsShared, nonSharedPartOwner: null); object? part = partLifecycle.GetValueReadyToExpose(); + isFullyInitialized = partLifecycle.State == PartLifecycleState.Final; return lookup.Export.MemberRef is null ? part : GetValueFromMember(part, exportingMember!, type, lookup.Export.ExportedValueTypeRef.Resolve()); diff --git a/test/Microsoft.VisualStudio.Composition.Tests/ThreadSafetyTests.cs b/test/Microsoft.VisualStudio.Composition.Tests/ThreadSafetyTests.cs index 91b4d4afe..eb484bc81 100644 --- a/test/Microsoft.VisualStudio.Composition.Tests/ThreadSafetyTests.cs +++ b/test/Microsoft.VisualStudio.Composition.Tests/ThreadSafetyTests.cs @@ -4,6 +4,7 @@ namespace Microsoft.VisualStudio.Composition.Tests { using System; + using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; @@ -744,6 +745,71 @@ public class GrandchildExport #endregion + #region Reentrant shared part publication + + [MefFact(CompositionEngines.V3EmulatingV2, typeof(ReentrantSharedPart))] + public async Task ReentrantSharedPartIsNotPublishedBeforeOnImportsSatisfiedCompletes(IContainer container) + { + ExportProvider exportProvider = container.GetExportedValue(); + var control = new ReentrantSharedPartControl(); + Assert.True(ReentrantSharedPart.Controls.TryAdd(exportProvider, control)); + + Task firstRequest = Task.Run(() => exportProvider.GetExportedValue()); + Task? concurrentRequest = null; + try + { + Assert.True(control.ReentrantQueryCompleted.Wait(TestUtilities.UnexpectedTimeout)); + concurrentRequest = Task.Run(() => + { + control.ConcurrentRequestStarted.Set(); + return exportProvider.GetExportedValue(); + }); + + Assert.True(control.ConcurrentRequestStarted.Wait(TestUtilities.UnexpectedTimeout)); + await Task.Delay(TestUtilities.ExpectedTimeout); + Assert.False(concurrentRequest.IsCompleted); + } + finally + { + control.AllowOnImportsSatisfiedToComplete.Set(); + ReentrantSharedPart.Controls.TryRemove(exportProvider, out _); + } + + ReentrantSharedPart firstResult = await firstRequest; + ReentrantSharedPart concurrentResult = await concurrentRequest; + Assert.Same(firstResult, concurrentResult); + } + + [Export, Shared] + public class ReentrantSharedPart + { + internal static readonly ConcurrentDictionary Controls = new ConcurrentDictionary(); + + [Import] + public ExportProvider ExportProvider { get; set; } = null!; + + [OnImportsSatisfied] + public void OnImportsSatisfied() + { + ReentrantSharedPartControl control = Controls[this.ExportProvider]; + ReentrantSharedPart self = this.ExportProvider.GetExportedValue(); + Assert.Same(this, self); + control.ReentrantQueryCompleted.Set(); + Assert.True(control.AllowOnImportsSatisfiedToComplete.Wait(TestUtilities.UnexpectedTimeout)); + } + } + + internal sealed class ReentrantSharedPartControl + { + internal ManualResetEventSlim ReentrantQueryCompleted { get; } = new ManualResetEventSlim(); + + internal ManualResetEventSlim ConcurrentRequestStarted { get; } = new ManualResetEventSlim(); + + internal ManualResetEventSlim AllowOnImportsSatisfiedToComplete { get; } = new ManualResetEventSlim(); + } + + #endregion + #region PartAlreadyFinalizedTest ///