PTAL @dotnet/jit-contrib

Will this work for a static span lifted to a local? Something like e.g.

private static ReadOnlySpan<byte> s_hexEncodeMap => new byte[] { (byte)'0', (byte)'1', (byte)'2', (byte)'3', (byte)'4', (byte)'5', (byte)'6', (byte)'7', (byte)'8', (byte)'9', (byte)'A', (byte)'B', (byte)'C', (byte)'D', (byte)'E', (byte)'F' };

/// <summary>
/// A faster version of String.Concat(<paramref name="str"/>, <paramref name="separator"/>, <paramref name="number"/>.ToString("X8"))
/// </summary>
/// <param name="str"></param>
/// <param name="separator"></param>
/// <param name="number"></param>
/// <returns></returns>
public static string ConcatAsHexSuffix(string str, char separator, uint number)
{
    var length = 1 + 8;
    if (str != null)
    {
        length += str.Length;
    }

    return string.Create(length, (str, separator, number), (buffer, tuple) =>
    {
        var (tupleStr, tupleSeparator, tupleNumber) = tuple;
        var hexEncodeMap = s_hexEncodeMap;

        var i = 0;
        if (tupleStr != null)
        {
            tupleStr.AsSpan().CopyTo(buffer);
            i = tupleStr.Length;
        }

        var number = (int)tupleNumber;
        buffer[i + 8] = (char)hexEncodeMap[number & 0xF];
        buffer[i + 7] = (char)hexEncodeMap[(number >> 4) & 0xF];
        buffer[i + 6] = (char)hexEncodeMap[(number >> 8) & 0xF];
        buffer[i + 5] = (char)hexEncodeMap[(number >> 12) & 0xF];
        buffer[i + 4] = (char)hexEncodeMap[(number >> 16) & 0xF];
        buffer[i + 3] = (char)hexEncodeMap[(number >> 20) & 0xF];
        buffer[i + 2] = (char)hexEncodeMap[(number >> 24) & 0xF];
        buffer[i + 1] = (char)hexEncodeMap[(number >> 28) & 0xF];
        buffer[i] = tupleSeparator;
    });
}

• Modification of dotnet/aspnetcore/.../StringUtilities.cs to use ReadOnlySpan<byte>

@benaadams yeah should work (just checked - no range checks for hexEncodeMap accesses)

Added PR then 😄 dotnet/aspnetcore#18406

How can you prove that the length of hexEncodeMap is 16 ?
The static s_hexEncodeMap can be re-assigned to be an array with a different length.

How can you prove that the length of hexEncodeMap is 16 ?

Its getter which is a look up into readonly data. Not sure it can be reassigned in anyway?

Not sure that there is anything that guarantees it is read only.
I know that reflection can be used to change "read only" values.
The JIT can't afford to omit a bounds check at allow an out of bounds read or write operation.

Its a method not a value; with the following il

.method private hidebysig specialname static 
	valuetype [System.Private.CoreLib]System.ReadOnlySpan`1<uint8> get_s_hexEncodeMap () cil managed 
{
	// Method begins at RVA 0x2050
	// Code size 13 (0xd)
	.maxstack 8

	IL_0000: ldsflda valuetype '<PrivateImplementationDetails>'/'__StaticArrayInitTypeSize=16' '<PrivateImplementationDetails>'::CE27CB141098FEB00714E758646BE3E99C185B71
	IL_0005: ldc.i4.s 16
	IL_0007: newobj instance void valuetype [System.Private.CoreLib]System.ReadOnlySpan`1<uint8>::.ctor(void*, int32)
	IL_000c: ret
}

Or following asm

C.get_s_hexEncodeMap()
    L0000: mov rax, 0x2d1c3a60844
    L000a: mov [rcx], rax
    L000d: mov dword [rcx+0x8], 0x10
    L0014: mov rax, rcx
    L0017: ret

So the length is fixed at 0x10 or 16 bytes

I see it is not an array but a [ReadOnly]Span which is a struct with a length field.

And to get this optimization, the JIT probably has to inline: get_s_hexEncodeMap and the call it makes to the ReadOnlySpan`1:.ctor as well.

Not sure I understand, its the same pattern as in the summary of the PR

static ReadOnlySpan<byte> span => new byte[] { 1,2,3,4 };

byte Case1(int i)  => span[i & 2];

How can you prove that the length of hexEncodeMap is 16 ?
The static s_hexEncodeMap can be re-assigned to be an array with a different length.

Well, jit already does that, see sharplab.io (thanks to Roslyn dotnet/roslyn#24621)
My PR only limits possible range of an indexer and doesn't remove anything by itself (it wouldn't remove anything if array's length was unknown)

Another improvement based on this change dotnet/aspnetcore#18450

@EgorBo, @dotnet/jit-contrib, what are the next steps for this PR? It's green, existing feedback appears to be addressed, and there's been no activity on it for a few weeks. Thanks.

@EgorBo, @dotnet/jit-contrib, what are the next steps for this PR? It's green, existing feedback appears to be addressed, and there's been no activity on it for a few weeks. Thanks.

it's ready for review from my end.
It handles the most common patterns I managed to find.

@dotnet/jit-contrib I think this can be merged. Anyone else wants to take a look at the final version?

Thank you @EgorBo !