Remove unsafe perf quirks from System.Text.Encodings.Web

[EgorBo]

Contributes to #94941 effort.

Should be ~zero performance difference as the JIT is smart enough to handle the safer equivalent, i.e.:

// New code:
static bool New(Span<byte> destination)
{
    return "&gt;"u8.TryCopyTo(destination);
}

// Old code:
static bool Old(Span<byte> destination)
{
    return TryWriteBytes(destination, (byte)'&', (byte)'g', (byte)'t', (byte)';');
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool TryWriteBytes(Span<byte> span, byte a, byte b, byte c, byte d)
{
    if (span.Length >= 4)
    {
        uint abcd32;
        if (BitConverter.IsLittleEndian)
        {
            abcd32 = ((uint)d << 24) | ((uint)c << 16) | ((uint)b << 8) | a;
        }
        else
        {
            abcd32 = ((uint)a << 24) | ((uint)b << 16) | ((uint)c << 8) | d;
        }
        Unsafe.WriteUnaligned<uint>(ref MemoryMarshal.GetReference(span), abcd32);
        return true;
    }
    else
    {
        return false;
    }
}

Codegen for New and Old:

; Method Program:New(System.Span`1[ubyte]):ubyte (FullOpts)
       mov      rax, bword ptr [rcx]
       mov      ecx, dword ptr [rcx+0x08]
       xor      edx, edx
       cmp      ecx, 4
       jl       SHORT G_M2274_IG04
       mov      dword ptr [rax], 0x3B746726
       mov      edx, 1
G_M2274_IG04:  ;; offset=0x0018
       mov      eax, edx
       ret      
; Total bytes of code: 27


; Method Program:Old(System.Span`1[ubyte]):ubyte (FullOpts)
       mov      rax, bword ptr [rcx]
       mov      ecx, dword ptr [rcx+0x08]
       cmp      ecx, 4
       jge      SHORT G_M16025_IG04
       xor      eax, eax
       jmp      SHORT G_M16025_IG05
G_M16025_IG04:
       mov      dword ptr [rax], 0x3B746726
       mov      eax, 1
G_M16025_IG05:
       ret      
; Total bytes of code: 27

Same for writing chars vs "str".TryCopyTo.

PS: SpanUtility.IsValidIndex can also be just removed (propagated at all uses) so we can remove the whole file, but it's out of scope for this PR since IsValidIndex is perfectly safe.

[dotnet-policy-service]

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

[EgorBo]

ugh, .NET Framework 😐

[MichalPetryka]

How does the performance compare with Mono?

[EgorBo]

How does the performance compare with Mono?

This is a part of the global security initiative to improve code safety. We assume that extra nanoseconds in these APIs for mono workloads won't be a big deal. Not sure performance matters for quite old System.Text.Encodings.Web at all, perhaps, only for HtmlEncoder.

[stephentoub]

This library builds for .NET Framework as well. Did these changes regress performance there? Do we care?

[EgorBo]

I guess it's impossible to avoid regressions on Mono and .NET Framework during "de-unsafization", the only part where it won't happen is the tiny coreclr-specific part of the corelib 😞

""<"u8.TryCopyTo" seems to be 5ns slower than the old hack on .NET Framework for me locally. But all the public APIs System.Text.Encodings.Web contains aren't fast anyway (e.g. UrlEncoder.Default.Encode takes ~ms) so hopefully it won't be noticed? I've got an impression that this package is not often used on hot path, that's why I started from it.

cc @jkotas (e.g. we recently introduced a similar regression for Mono in #108572 (comment))