Replace String concat with StringBuilder in Cluster Nodes

[msft-paddy14]

StackExchange.Redis does periodic CLUSTER NODES and for large clusters a lot of allocations can happen with such calls.
We recently saw an issue where lots of allocation increased our .NET committed memory and also GC activity when new clients would be deployed/control plane would probe nodes where cluster size was ~100 nodes.

Using StringBuilder in an adhoc BDN test shows we can get significant improvements using this

| TestMultiStringConcat | None | 125.13 us | 1.894 us | 2.395 us | 93.5059 | 4.3945 | 1567640 B |
| TestMultiStringConcatBuilder | None | 41.59 us | 0.627 us | 0.586 us | 9.5215 | 0.9155 | 160152 B |

[badrishc]

Ideally, there would be no need to create strings at all, or use stringbuilder, if this is a very common operation. We could directly write to a session-local scratch buffer that is reusable for that session, and write to it a sequence of the usual style of writing:

while (!RespWriteUtils.TryWriteXxx(...))
   ...

Here is a rough (non-working) prototype of this for CLUSTER NODE: https://github.com/microsoft/garnet/compare/badrishc/cluster-node?expand=1

However, the StringBuilder approach is reasonable and simple to begin with, if you are seeing it is sufficient for your use case.

[msft-paddy14]

Ideally, there would be no need to create strings at all, or use stringbuilder, if this is a very common operation. We could directly write to a session-local scratch buffer that is reusable for that session, and write to it a sequence of the usual style of writing:

while (!RespWriteUtils.TryWriteXxx(...))
   ...

Here is a rough (non-working) prototype of this for CLUSTER NODE: https://github.com/microsoft/garnet/compare/badrishc/cluster-node?expand=1

However, the StringBuilder approach is reasonable and simple to begin with, if you are seeing it is sufficient for your use case.

Your approach seems more performant. However, since it's relatively more code and needs few interface changes, maybe string builder is okay to start with for simplicity and more gains compared to main code. We can optimize further with scratchbuffer if needed?

[badrishc]

Ideally, there would be no need to create strings at all, or use stringbuilder, if this is a very common operation. We could directly write to a session-local scratch buffer that is reusable for that session, and write to it a sequence of the usual style of writing:

while (!RespWriteUtils.TryWriteXxx(...))
   ...

Here is a rough (non-working) prototype of this for CLUSTER NODE: https://github.com/microsoft/garnet/compare/badrishc/cluster-node?expand=1
However, the StringBuilder approach is reasonable and simple to begin with, if you are seeing it is sufficient for your use case.

Your approach seems more performant. However, since it's relatively more code and needs few interface changes, maybe string builder is okay to start with for simplicity and more gains compared to main code. We can optimize further with scratchbuffer if needed?

Yeah this is at least better than main.

[badrishc]

use .Append(char c) for all single characters such as ' ', ':', etc.

[badrishc]

Pass the current StringBuilder to GetSlotRange and GetSpecialStates so we are not creating new string builders or intermediate strings.

[badrishc]

i.e. we can call GetSlotRange(workerId, nodeInfoStringBuilder) here.

[badrishc]

Send this StringBuilder to private void GetNodeInfo(ushort workerId, ConnectionInfo info, StringBuilder sb) so it can append to it.

And implement the public string GetNodeInfo(ushort workerId, ConnectionInfo info) using the common method, as something like:

var sb = new StringBuilder()
GetNodeInfo(..., sb);
return sb.ToString();