Skip to content

Conversation

MihaZupan
Copy link
Member

@MihaZupan MihaZupan commented Sep 12, 2025

Polyfilling such helpers seems like a neat way of avoiding duplication in a bunch of places where we might want to use newer APIs.

W3cPropagator being an extreme example where we were already duplicating a bunch of logic, but still not using SearchValues everywhere because it would add even more.

Benchmark source
public class ActivityBench
{
    private static readonly DistributedContextPropagator s_propagator = DistributedContextPropagator.CreateW3CPropagator();
    private readonly Activity _activity = new Activity("Test");

    public ActivityBench()
    {
        _activity.AddBaggage("a", "aaaaabbbbbcccccddddd");
        _activity.Start();
    }

    [Benchmark]
    public void Extract() => s_propagator.ExtractTraceIdAndState(null, static (carrier, name, out value, out values) =>
    {
        value = name == "traceparent" ? "00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-01" : null;
        values = null;
    }, out _, out _);

    [Benchmark]
    public void InjectBaggage() => s_propagator.Inject(_activity, null, static (carrier, name, value) => { });
}
Method Toolchain Mean Error Ratio
Extract \main\corerun.exe 27.15 ns 0.407 ns 1.00
Extract \pr\corerun.exe 9.75 ns 0.133 ns 0.36
InjectBaggage \main\corerun.exe 55.39 ns 1.121 ns 1.00
InjectBaggage \pr\corerun.exe 44.18 ns 0.730 ns 0.80

@MihaZupan MihaZupan added this to the 11.0.0 milestone Sep 12, 2025
@MihaZupan MihaZupan self-assigned this Sep 12, 2025
@Copilot Copilot AI review requested due to automatic review settings September 12, 2025 23:07
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 12, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds polyfills for modern .NET APIs like SearchValues to enable optimization of string searching operations across different target frameworks. The key change is replacing many manual character-by-character validation loops and character array lookups with more efficient SearchValues operations.

Key changes:

  • Adds polyfill infrastructure for SearchValues<char>, Ascii.IsValid, and span extension methods for downlevel targets
  • Optimizes W3CPropagator by replacing bitmask-based validation with SearchValues and simplifying trace parent validation logic
  • Converts various libraries to use SearchValues instead of IndexOfAny with character arrays or manual loops

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

File Description
src/libraries/Directory.Build.targets Adds MSBuild logic to include polyfill files when IncludeSpanPolyfills is enabled
src/libraries/Common/src/System/Text/AsciiPolyfills.cs Polyfill implementation of Ascii.IsValid methods for downlevel targets
src/libraries/Common/src/System/MemoryExtensionsPolyfills.cs Polyfill for ContainsAnyExcept span extension method
src/libraries/Common/src/System/Buffers/SearchValuesPolyfills.cs Complete polyfill implementation of SearchValues APIs using bitmap for ASCII chars
Multiple library projects Enable polyfills and convert to use SearchValues instead of manual validation loops

@MihaZupan MihaZupan force-pushed the searchvalues-polyfills2 branch from f823ab1 to 6ab683b Compare September 13, 2025 02:11
@MihaZupan MihaZupan requested a review from sbomer as a code owner September 13, 2025 02:11
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Sep 13, 2025
@MihaZupan MihaZupan added area-System.Diagnostics.Activity and removed linkable-framework Issues associated with delivering a linker friendly framework needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Sep 13, 2025
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Sep 13, 2025
Copy link
Contributor

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

@MihaZupan MihaZupan added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) and removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) labels Sep 16, 2025
@MihaZupan MihaZupan force-pushed the searchvalues-polyfills2 branch from cea32e2 to 190cb7d Compare September 17, 2025 13:11
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MihaZupan MihaZupan merged commit a09317d into dotnet:main Oct 10, 2025
107 of 109 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Meta linkable-framework Issues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants