Skip to content

Cache the culture sensitivity of the types - #1396

Merged
meziantou merged 1 commit into
mainfrom
feature/culture-sensitive-caching-c0516a
Sep 6, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/culture-sensitive-caching-c0516a

Conversation

@meziantou

Copy link
Copy Markdown
Owner

What

CultureSensitiveFormattingContext.GetCultureSensitivity(ITypeSymbol, CultureSensitiveOptions) is a pure function of the type symbol and the options, but nothing was cached: it was fully recomputed for every node.

Each call runs about twenty symbol comparisons plus several IsOrInheritsFrom / IsOrImplements hierarchy walks, and reaches HasToStringWithFormatProvider, which was type.GetAllMembers().OfType<IMethodSymbol>().Any(...) — a yield return iterator enumerating every member of every type in the base chain, wrapped in two more LINQ iterators, with a per-member IsOrInheritsFrom on the parameter type.

Four analyzers enabled by default drive this (UseIFormatProviderAnalyzer, DoNotUseImplicitCultureSensitiveToStringAnalyzer, DoNotUseToStringIfObjectAnalyzer, SimplifyStringCreateWhenAllParametersAreCultureInvariantAnalyzer), so it ran on every invocation, interpolated string and binary operation of a compilation, and it is worst on codebases with deep type hierarchies where the member walk is longest.

Two changes:

  • Memoize the result in a ConcurrentDictionary keyed by the type symbol and the options, the same way AwaitableTypes caches IsAwaitable. The body was split into a cached front door plus GetCultureSensitivityCore.
  • Replace the LINQ chain in HasToStringWithFormatProvider with a plain loop over GetMembers(nameof(ToString)) up the base chain.

Notes for the reviewer

  • Purity. The only input besides (typeSymbol, options) is compilation, which is fixed for the lifetime of a context instance, so the answer is stable for a compilation. IsCultureSensitiveTypeUsingAttribute reads compilation.Assembly.GetAttributes() and is pure for the same reason.
  • The cache key uses the full options flags, not only the UnwrapNullableOfT flag the method reads today. The flags are bounded to sixteen combinations, so the extra entries cost nothing, and the cache stays correct if another flag starts to matter. UnwrapNullableOfT genuinely has to be part of the key: with it, int? returns the sensitivity of int, without it the one of Nullable<int>.
  • TryGetValue + indexer rather than GetOrAdd(key, factory), to avoid a delegate allocation on every lookup — the GetOrAdd<TArg> overload that would avoid a closure is not available on netstandard2.0. The computation is idempotent, so a race just recomputes.
  • The HasToStringWithFormatProvider rewrite is equivalent, not a behavior change. The GetAllMembers() overload without a name walks the base chain only, so the loop matches it. The GetAllMembers(name) overload would not have been equivalent: it additionally walks AllInterfaces for interface symbols, which would have made an interface inheriting a ToString(IFormatProvider) report CultureSensitive instead of MaybeCultureSensitiveOpaqueRuntimeType, since IsFormattableType is checked before IsOpaqueRuntimeType.
  • Sharing a single context across the four analyzers was considered and left out: it would need a compilation-keyed static cache with its own lifetime concerns, for about 116 GetBestTypeByMetadataName calls per compilation — negligible next to the per-node work this removes.

Testing

  • All five Roslyn versions build (4.8, 4.14, 5.0, 5.6, 5.9), 0 warnings and 0 errors.
  • dotnet test --max-parallel-test-modules 2 over the five test projects: 19258 passed, 0 failed.
  • dotnet run --project src/DocumentationGenerator exits 0 with no markdown change: the change is internal and does not affect any rule behavior or documentation.

CultureSensitiveFormattingContext.GetCultureSensitivity(ITypeSymbol,
CultureSensitiveOptions) is a pure function of the type symbol and the
options, but it was recomputed for every node. It runs about twenty symbol
comparisons and several hierarchy walks, and reaches
HasToStringWithFormatProvider, which enumerated every member of every type
of the base chain through LINQ iterators. Four analyzers that are enabled
by default drive it, so this ran on every invocation, interpolated string
and binary operation of a compilation.

Memoize the result in a ConcurrentDictionary keyed by the type symbol and
the options, the same way AwaitableTypes caches IsAwaitable. The key uses
the full options flags rather than the single flag the method reads today:
the flags are bounded to sixteen combinations, so it costs nothing and
remains correct if another flag starts to matter.

Also replace the LINQ chain of HasToStringWithFormatProvider with a loop
over GetMembers("ToString") up the base chain. This is equivalent, as the
GetAllMembers() overload without a name walks the base chain only.
@meziantou
meziantou merged commit 7de6b32 into main Sep 6, 2026
13 checks passed
@meziantou
meziantou deleted the feature/culture-sensitive-caching-c0516a branch September 6, 2026 04:16
This was referenced Sep 6, 2026
This was referenced Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant