Redesign CallSiteFactory constructor selection with stable arity ordering and parameter-aware ambiguity

[Copilot]

CallSiteFactory.CreateConstructorCallSite no longer relies on unstable Array.Sort behavior for multi-constructor selection. This update reworks selection to be deterministic and allocation-lighter, and it strengthens ambiguity analysis for keyed/default-resolution scenarios.

• What changed

  • Replaced sort-and-walk with a fused insertion pass that builds a stable arity-desc constructor order while calling GetParameters() once per constructor.
  • Establishes the first resolvable constructor in arity-desc order as best and retains its ServiceCallSite[] for activation.
  • Replaced type-only ambiguity tracking with parameter-aware tracking using ServiceIdentifier plus resolution origin:
    • resolved from container/call-site
    • resolved from default value
  • Added probe-based resolution for non-selected constructors to avoid materializing ServiceCallSite[] when not needed.
  • Preserved constructors.Length == 0 and constructors.Length == 1 short-circuit behavior and existing exception message shapes.

• Behavioral delta (intended and scoped)

  • Same-arity identical parameter-type-set ties are deterministic (stable declaration order within equal arity).
  • Cross-arity disjoint resolvable constructors now consistently throw ambiguity instead of producing order-dependent outcomes.
  • Ambiguity detection now distinguishes keyed-service identity (ServiceIdentifier key + type), instead of collapsing to Type only.
  • Ambiguity probing now distinguishes default-origin vs container-origin resolution for matching service identifiers.

• Tests added

  • Same-arity, identical parameter-type set, different parameter order → verifies deterministic constructor selection.
  • Same-arity, disjoint parameter types, both resolvable → verifies ambiguous-constructor InvalidOperationException.
  • Cross-arity disjoint resolvable constructors in both declaration orders → verifies both throw ambiguity.
  • Same parameter Type with different [FromKeyedServices] keys → verifies keyed identity is treated distinctly.
  • Best constructor using default-origin parameter with smaller constructor requiring container resolution for same identifier → verifies smaller constructor is ignored as non-resolvable.
  • Short resolvable declared before longer resolvable / longer non-resolvable coverage retained.

// Before: unstable ordering + repeated GetParameters allocations
// Array.Sort(constructors, (a, b) => b.GetParameters().Length.CompareTo(a.GetParameters().Length));

// After: fused insertion ordering + parameter-aware ambiguity probing
var sortedConstructors = new ConstructorInfo[constructors.Length];
var sortedParameters = new ParameterInfo[constructors.Length][];

for (int i = 0; i < constructors.Length; i++)
{
    ConstructorInfo constructor = constructors[i];
    ParameterInfo[] parameters = constructor.GetParameters();
    int sortedIndex = i;

    while (sortedIndex > 0 && sortedParameters[sortedIndex - 1].Length < parameters.Length)
    {
        sortedConstructors[sortedIndex] = sortedConstructors[sortedIndex - 1];
        sortedParameters[sortedIndex] = sortedParameters[sortedIndex - 1];
        sortedIndex--;
    }

    sortedConstructors[sortedIndex] = constructor;
    sortedParameters[sortedIndex] = parameters;
}

• DI suite status
  • In this environment, targeted DI build/test invocations were attempted but blocked by missing shared framework/targeting-pack prerequisites (eng/targetingpacks.targets).
  • Secret scan reported no secrets in changed files.
  • CodeQL was invoked; analysis returned no alerts but was skipped due database size limits in this environment.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

[rosebyte]

@copilot, the change can be more ambitious, please reimplement using this design:

Constructor selection in CallSiteFactory: redesign proposal

1. Context

Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.CreateConstructorCallSite
(file src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs)
is invoked once per (serviceType, implementationType) pair when the container
builds a call site for a service. The result is cached in _callSiteCache, so
the cost is paid once per pair across the lifetime of the container.

The trivial paths for constructors.Length == 0 and constructors.Length == 1
are already short-circuited; everything below applies only when an
implementation type exposes two or more public constructors.

1.1 The selection rule (informal)

1. A constructor is resolvable iff CreateArgumentCallSites returns
   non-null for it, i.e. every parameter is either:
   • satisfied by a container registration (possibly keyed via
     [FromKeyedServices] / [ServiceKey]), or
   • carries a compile-time default (HasDefaultValue == true).
2. Among resolvable constructors, the one with the largest
   parameters.Length wins (greedy-longest).
3. If two resolvable constructors have the same arity but neither is a
   subset of the other's parameter type set, the choice is ambiguous and
   InvalidOperationException("The following constructors are ambiguous:...")
   is thrown.

1.2 What "resolvable" actually depends on

Parameter declaration	HasDefaultValue	Always resolvable?
IFoo foo	false	No, needs registration
IFoo? foo (NRT only)	false	No, needs registration
IFoo foo = null	true	Yes, injects null
IFoo? foo = null	true	Yes, injects null
int n	false	No
int n = 42	true	Yes
int? n	false	No
int? n = null	true	Yes
MyStruct s = default	true	Yes (via GetUninitializedObject)
MyEnum? e = MyEnum.Foo	true	Yes (re-boxed)

C# 8 nullable reference annotations alone have no effect on DI
resolvability; they are not visible to reflection's HasDefaultValue.

2. Problems with the current code on main

Array.Sort(constructors,
    (a, b) => b.GetParameters().Length.CompareTo(a.GetParameters().Length));

1. Array.Sort is unstable. When two constructors have the same arity the
   "winner" depends on the sort algorithm's internal state, the array length,
   the runtime, and the .NET version. Same-arity-identical-type-set ties pick
   an indeterminate constructor; same-arity-disjoint ties throw
   AmbiguousConstructorException with an indeterminate pair in the message.

2. GetParameters() is called inside the comparator. GetParameters() is
   a reflection call that allocates a fresh ParameterInfo[] per invocation.
   Sorting calls it ~2·N·log N times, then the loop calls it again per
   constructor (N), then the hash-set build calls it once more on
   bestConstructor. Total: roughly 2·N·log N + N + 1 allocations of
   ParameterInfo[] per selection.

3. The ambiguity model is impoverished. bestConstructorParameterTypes is
   a HashSet<Type>. This conflates:

   • parameters best resolved via container vs via default, and
   • parameters with different [FromKeyedServices(...)] keys but the same
     ParameterType.

3. Problems with PR #129119's approach

PR #129119 replaces Array.Sort with a declaration-order single pass, caches
GetParameters(), and introduces "supersede + recheck" plus deferred
ambiguity tracking.

3.1 Algorithmic worst case

Walking in declaration order means a smaller ctor can become best first and
then be superseded by a larger ctor later. Each supersede:

• calls CreateArgumentCallSites on the new larger candidate, and
• re-walks resolvedConstructors to recompute ambiguity against the new
  best.types.

In the LargestLast shape (longest constructor declared last), this
degrades to: O(N) CreateArgumentCallSites calls, plus repeated
resolvedConstructors walks. The benchmark below shows this is only ~10%
faster than the unstable-sort baseline.

3.2 Order-dependent behavioural drift ("Example F")

public Foo(IA a, IB b, IC c = null) { }   // arity 3, always resolvable
public Foo(ID d = null) { }               // arity 1, always resolvable, disjoint

Declaration order	Old code	PR #129119
3-arg first	throws ambiguous	silently picks 3-arg
1-arg first	throws ambiguous	throws ambiguous

The PR's behaviour depends on source order, which is undesirable.

3.3 Impoverished ambiguity model unchanged

The PR still uses HasSet<Type> for bestConstructorParameterTypes and so
inherits the conflation of container-vs-default and the keyed-services blind
spot from the old code.

4. Proposed design

A four-step change that, taken together, beats both the current code and the
PR on every measured shape while having simpler control flow and a stronger
behaviour contract.

4.1 Step 1: fused insertion sort during a single build pass

Replace the sort-then-walk structure with a single pass that inserts each
constructor into its sorted position as GetParameters() is called for it.
This:

• removes the comparator delegate (no closure allocation),
• calls GetParameters() exactly N times (down from 2·N·log N + N + 1),
• produces an arity-DESC, declaration-stable order via the natural stability
  of insertion sort with strictly shorter comparison,
• does not require an int[] order scratch array.

var sortedCtors  = new ConstructorInfo[constructors.Length];
var sortedParams = new ParameterInfo[constructors.Length][];

for (int i = 0; i < constructors.Length; i++)
{
    ConstructorInfo c = constructors[i];
    ParameterInfo[] p = c.GetParameters();

    int j = i;
    while (j > 0 && sortedParams[j - 1].Length < p.Length)
    {
        sortedCtors[j]  = sortedCtors[j - 1];
        sortedParams[j] = sortedParams[j - 1];
        j--;
    }
    sortedCtors[j]  = c;
    sortedParams[j] = p;
}

For the typical N ≤ 10 this is faster than Array.Sort (which uses
insertion sort internally below the introsort threshold of 16, but with
comparator delegate overhead).

4.2 Step 2: skip-smaller pruning

Because we walk in arity-DESC order, the first resolvable constructor we
encounter is the final best by arity. Every constructor of strictly lower
arity is either absorbed (subset rule) or ambiguous. Smaller resolvable
constructors are never selected, so we don't need to materialise their
ServiceCallSite[].

for (int i = 0; i < n; i++)
{
    var p = sortedParams[i];
    if (best is not null && p.Length < bestLen) continue;   // skip resolve
    ...
}

This combined with Step 1 means we resolve only the top arity tier, plus
any smaller resolvable ctors whose ambiguity we need to verify (see Step 3).

4.3 Step 3: parameter-aware ambiguity model

Replace HashSet<Type> bestConstructorParameterTypes with a richer
representation that captures both service identity (so keyed services
disambiguate correctly) and resolution origin (so default-vs-container
distinguishes correctly):

// Reusing the existing ServiceIdentifier (Key, ServiceType) tuple.
HashSet<ServiceIdentifier> bestFromContainer = ...;   // T_container
HashSet<ServiceIdentifier> bestFromDefault   = ...;   // T_default

Population happens when best is established (or replaced after Step 1 +
Step 2, which actually means "when best is first selected", because Step 1
guarantees no upgrades). For each parameter bp of best:

• if CreateArgumentCallSites resolved it from the container → add to
  bestFromContainer;
• if from ConstantCallSite synthesised by TryGetDefaultValue → add to
  bestFromDefault.

For each subsequent smaller ctor X, the existing CreateArgumentCallSites
call is replaced by a per-parameter probe:

bool hasNewResolvable = false;
foreach (var Pi in X.parameters)
{
    var id = ServiceIdentifierFor(Pi);              // honours keyed attributes
    if (bestFromContainer.Contains(id)) continue;   // resolvable via container
    if (bestFromDefault.Contains(id))
    {
        if (Pi.HasDefaultValue) continue;           // X resolves via its OWN default
        return /* abandon X, not resolvable */;
    }
    // id ∉ T → probe Pi individually (single GetCallSite, no array alloc)
    if (probeResolvable(Pi))
        hasNewResolvable = true;                    // candidate ambiguity, keep walking
    else
        return /* abandon X, not resolvable */;
}

if (hasNewResolvable)
    throw Ambiguous(best, X);
// else: X is fully a subset of best, absorbed silently

This preserves the old code's behaviour exactly in all the previously
non-anomalous cases, and additionally:

• correctly handles keyed services (no longer collapses different keys onto
  the same Type);
• avoids allocating a ServiceCallSite[] for smaller-tier ctors that are not
  going to be used.

4.4 Step 4: ambiguity throws immediately

Because Step 1 + Step 2 mean best is established once and never upgraded,
there is no need for the PR's deferred-ambiguity bookkeeping
(ambiguousConstructor, bestConstructorForAmbiguousConstructor,
resolvedConstructors). Ambiguity throws on the spot, matching the old
code.

5. Behaviour invariants

Property	Old code	PR #129119	This design
Greedy-longest selection	✓	✓	✓
Subset absorption	✓	✓	✓
Throws on same-arity disjoint resolvable	✓ (indeterminate pair)	✓ (order-dependent pair)	✓ (deterministic pair)
Tie on same-arity identical type set	indeterminate winner	first in declaration order	first in declaration order
Cross-arity disjoint resolvable ("Example F")	always throws	order-dependent	always throws
Defaults vs container origin distinguished	no	no	yes
Keyed-services key distinguished	no	no	yes

The two bold rows are intentional improvements over both predecessors. The
Example F row restores old-code behaviour (which the PR breaks).

6. Empirical evidence

Microbenchmark over four algorithm variants, simulating
CreateArgumentCallSites with a HashSet<Type> lookup per parameter plus
object[] allocation. Apple M4 Max, .NET 10.0.5, 10 iterations / 5 warmup.

                            Mean (ns)    Ratio vs V1
─── TwoSameArity (PR motivating tie) ─────────────
V1 OldUnstableSort          142.2         1.00
V2 PR DeclarationOrder       98.4         0.69
V3 StableSort + Skip         99.5         0.70
V4 FusedInsertion (this)     94.1         0.66    ← win

─── LargestFirst (PR best case) ──────────────────
V1 OldUnstableSort          227.2         1.00
V2 PR DeclarationOrder       74.5         0.33
V3 StableSort + Skip         80.3         0.35
V4 FusedInsertion (this)     69.7         0.31    ← win

─── LargestLast (PR worst case) ──────────────────
V1 OldUnstableSort          233.2         1.00
V2 PR DeclarationOrder      208.8         0.90
V3 StableSort + Skip         81.6         0.35
V4 FusedInsertion (this)     74.7         0.32    ← win, 2.8× over PR

─── FiveScattered (realistic shape) ──────────────
V1 OldUnstableSort          473.1         1.00
V2 PR DeclarationOrder      255.9         0.54
V3 StableSort + Skip        113.2         0.24
V4 FusedInsertion (this)    113.1         0.24    ← tied; lowest alloc

─── LongestUnresolvable ──────────────────────────
V1 OldUnstableSort          286.3         1.00
V2 PR DeclarationOrder      211.9         0.74
V3 StableSort + Skip        111.9         0.39
V4 FusedInsertion (this)    107.6         0.38    ← win

Caveats:

• Real CreateArgumentCallSites performs recursive GetCallSite calls;
  costs are higher than the mock used, which widens the gap further
  (more expensive resolve = more savings from skipping).
• Allocations for V4 are consistently the lowest across all shapes
  (e.g. LargestLast: V4 = 312 B vs V2 = 1000 B).
• OneCtor (constructors.Length == 1) is excluded; it is short-circuited
  before any selection logic and so measures only method-shape JIT artefacts.

7. Reference implementation sketch

private ConstructorCallSite CreateConstructorCallSite(
    ResultCache lifetime,
    ServiceIdentifier serviceIdentifier,
    [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]
    Type implementationType,
    CallSiteChain callSiteChain)
{
    try
    {
        callSiteChain.Add(serviceIdentifier, implementationType);
        ConstructorInfo[] constructors = implementationType.GetConstructors();

        if (constructors.Length == 0)
            throw new InvalidOperationException(SR.Format(SR.NoConstructorMatch, implementationType));

        if (constructors.Length == 1)
            return BuildSingleCtorCallSite(lifetime, serviceIdentifier,
                                           implementationType, callSiteChain, constructors[0]);

        // Step 1: fused insertion-sort during single pass.
        int n = constructors.Length;
        var sortedCtors  = new ConstructorInfo[n];
        var sortedParams = new ParameterInfo[n][];
        for (int i = 0; i < n; i++)
        {
            ConstructorInfo c = constructors[i];
            ParameterInfo[] p = c.GetParameters();
            int j = i;
            while (j > 0 && sortedParams[j - 1].Length < p.Length)
            {
                sortedCtors[j]  = sortedCtors[j - 1];
                sortedParams[j] = sortedParams[j - 1];
                j--;
            }
            sortedCtors[j]  = c;
            sortedParams[j] = p;
        }

        // Step 2 + 3 + 4: walk arity-DESC, skip smaller, parameter-aware ambiguity.
        ConstructorInfo? best = null;
        ServiceCallSite[]? bestSites = null;
        int bestLen = -1;
        HashSet<ServiceIdentifier>? bestFromContainer = null;
        HashSet<ServiceIdentifier>? bestFromDefault   = null;

        for (int i = 0; i < n; i++)
        {
            ParameterInfo[] p = sortedParams[i];
            if (best is not null && p.Length < bestLen) continue;

            if (best is null)
            {
                // Establish best. Full CreateArgumentCallSites is needed here
                // because we will keep these call sites.
                var sites = CreateArgumentCallSites(
                    serviceIdentifier, implementationType, callSiteChain, p,
                    throwIfCallSiteNotFound: false);
                if (sites is null) continue;

                best = sortedCtors[i];
                bestSites = sites;
                bestLen = p.Length;
                ClassifyBestParameters(p, sites, out bestFromContainer, out bestFromDefault);
            }
            else
            {
                // Same-arity candidate or smaller (smaller is filtered above).
                if (!ProbeSmallerOrSameArity(p, bestFromContainer!, bestFromDefault!,
                                             callSiteChain, serviceIdentifier, out bool hasNewResolvable))
                    continue;                                  // X not resolvable, ignored
                if (hasNewResolvable)
                    throw Ambiguous(implementationType, best, sortedCtors[i]);
            }
        }

        if (best is null)
            throw new InvalidOperationException(SR.Format(SR.UnableToActivateTypeException, implementationType));

        return new ConstructorCallSite(lifetime, serviceIdentifier.ServiceType,
                                       best, bestSites!, serviceIdentifier.ServiceKey);
    }
    finally
    {
        callSiteChain.Remove(serviceIdentifier);
    }
}

ClassifyBestParameters walks bestParams paired with the just-built
sites, classifying each as container-origin (call site comes from
GetCallSite) or default-origin (call site is the ConstantCallSite
synthesised by TryGetDefaultValue).

ProbeSmallerOrSameArity implements the per-parameter probe from §4.3,
returning false if the candidate is not resolvable and true otherwise
(with hasNewResolvable indicating whether to throw ambiguous).

Ambiguous builds the existing
SR.AmbiguousConstructorException message with the deterministic
(best, candidate) pair.

8. Risks and open questions

1. Keyed-services behaviour change. Today's HashSet<Type> treats
   [FromKeyedServices("a")] IFoo and [FromKeyedServices("b")] IFoo as the
   same parameter for ambiguity purposes. The proposed design treats them as
   distinct. This is a behaviour change. It is probably correct, but it
   may break existing tests that lock in the current behaviour. Decision
   needed: fix-the-bug or preserve-the-bug.

2. Example F restoration. The proposal restores old-code behaviour
   ("always throw on cross-arity disjoint resolvable"). The PR's
   order-dependent behaviour will be lost. This is intentional, but worth
   stating explicitly in the PR description.

3. ProbeSmallerOrSameArity helper. The probe needs access to
   GetCallSite(ServiceIdentifierFor(Pi), callSiteChain) without allocating
   a ServiceCallSite[]. Either a new helper method, or a bool TryProbe
   overload of CreateArgumentCallSites. The shape of this helper is a
   small public-API decision (internal API, but still worth one round of
   review).

4. OneCtor short-circuit retention. Keep the existing
   constructors.Length == 1 early return. It is not a property of any of
   the algorithm variants; it is a separate optimisation that all of them
   inherit.

5. Testing. Existing PR-129119 tests should pass unchanged. Additionally,
   add tests for:

   • Cross-arity disjoint resolvable in both source orders → both throw
     ambiguous (Example F).
   • Keyed-services parameter identity (two ctors with same Type but
     different [FromKeyedServices] key) → behaves as if types were
     distinct.
   • Default-origin parameter in best whose Type reappears as a
     non-defaulted parameter in a smaller ctor → smaller ctor is
     non-resolvable, silently ignored (no ambiguous throw).

9. Summary

Aspect	Current	PR #129119	This proposal
Sort	unstable Array.Sort	none (declaration walk)	fused insertion sort
GetParameters() calls	~2·N·log N + N + 1	N	N
Resolves called	up to N	1 to N (often more than this proposal)	top arity tier only
Per-smaller-ctor ServiceCallSite[] alloc	yes	yes	no (probe-only)
Ambiguity model	type-set	type-set	identity + origin
Keyed-services correct	no	no	yes
Default-vs-container distinguished	no	no	yes
Example F behaviour	throws	order-dependent	throws
Allocations (LargestLast)	1000 B	1000 B	312 B
Time vs old (worst case)	1.00	0.90	0.32

[Copilot]

@copilot, the change can be more ambitious, please reimplement using this design:

Constructor selection in CallSiteFactory: redesign proposal

...

Implemented this redesign in commit d33c5b87:

• switched to fused insertion ordering (arity-desc, stable for equal arity)
• added parameter-aware ambiguity probing using ServiceIdentifier + resolution origin (call-site vs default)
• removed deferred ambiguity bookkeeping and now throw ambiguity immediately
• added tests for cross-arity disjoint ambiguity in both declaration orders, keyed-service identity distinction, and default-vs-container origin behavior.

I also ran secret scanning (clean) and CodeQL (no alerts produced; analysis was skipped due database size in this environment).