Don't allocate method name strings in protocol

[davidfowl]

• Today SignalR allocates a string when parsing the incoming protocol message, this change introduces a new API on IInvocationBinder that lets us avoid that allocation by asking the invocation provider for the string.
• This can be implemented on the client as well, that's not in this PR.

Performance looks good:

Method	Mean	Error	StdDev	Gen 0	Allocated
StringLookup	174.5 ns	8.16 ns	24.07 ns	0.0050	32 B
Uf8Lookup	178.7 ns	7.66 ns	22.58 ns	-	-

Code here:

Details

using System.Text.Json;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkRunner.Run<StringLookupBenchmark>();

[MemoryDiagnoser]
public class StringLookupBenchmark
{
    private readonly Dictionary<string, string> _cached = new(StringComparer.OrdinalIgnoreCase)
    {
        ["Echo"] = "Echo"
    };

    private readonly Utf8HashLookup _utf8Lookup = new Utf8HashLookup();

    public StringLookupBenchmark()
    {
        _utf8Lookup.Add("Echo");
    }

    private static ReadOnlySpan<byte> Payload => "{\"target\": \"echo\"}"u8;

    [Benchmark]
    public string StringLookup()
    {
        var reader = new Utf8JsonReader(Payload);
        reader.Read(); // Start object
        reader.Read(); // property name
        reader.Read(); // property value
        var target = reader.GetString()!;
        reader.Read(); // end object
        _cached.TryGetValue(target, out var value);
        return value!;
    }

    [Benchmark]
    public string Uf8Lookup()
    {
        var reader = new Utf8JsonReader(Payload);
        reader.Read(); // Start object
        reader.Read(); // property name
        reader.Read(); // property value
        var target = reader.ValueSpan;
        reader.Read(); // end object
        _utf8Lookup.TryGetValue(target, out var value);
        return value!;
    }
}

Fixes #41342

[davidfowl]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[halter73]

Can we do the UTF8 encoding here during initialization once per method instead of for every invocation in TryGetValue? Case-insensitive comparisons might be harder this way, but could we just check for ordinal equality. How common is it really to use different casing for the method names on the server? A cache miss costs an allocation, but the upside is saving the encoding step in the common case. I know the encoding is usually just a simple widen in practice, but it seems worth it to avoid it if we can.

[davidfowl]

Not worth it. The performance isn't bad enough to spend time on this.

I will add that I started by converting to utf8 and case insensitivity lead me to revert back to utf16. When using the javascript clients its common to not match the casing on the server and all of the efficient methods for lower case sensitive comparison is utf16 based.

[halter73]

Are key and value ever different?

[davidfowl]

Yes, the casing.

[halter73]

I don't think they are different:

https://github.com/dotnet/aspnetcore/pull/41465/files#diff-dfbf204b25e7056f036ce6b4c77a324707532c5757084b0fbae3de2789b9b409R41-R42

Is this getting reassigned anywhere? I don't see it.

[davidfowl]

Ah yes, I can remove that. I used to ToLowerInvariant the key when I made the initial change.

[halter73]

@davidfowl I wish I noticed you enabled auto merge 😞. Not a huge deal, but are you going to submit a follow up?

[davidfowl]

I have no plans to further optimize this no 😄