Unnecessary allocations by invoking CommunicationsUtilities.Trace

[MichalPavlik]

CommunicationsUtilities.Trace defines format and arguments for String.Format implementation. Although String.Format implements overloads for 1, 2 and 3 arguments without object[] allocation, we are missing these options in CommunicationsUtilities.

[KalleOlaviNiemitalo]

The arguments are often integers that will still be boxed if you add overloads for 1, 2, and 3 objects. When tracing is disabled, that boxing could perhaps be avoided by using a C# 10 custom string interpolation handler, or by making callers check Traits.Instance.DebugNodeCommunication. Many of those CommunicationsUtilities.Trace calls are in error handling though, and are likely not important for performance.

[Forgind]

Many of those CommunicationsUtilities.Trace calls are in error handling though, and are likely not important for performance.

...and most of the rest aren't hit too many times per build.

That said, this should be a fairly quick fix, and it would still help, so I'm supportive.

[MichalPavlik]

I will have to measure, how many allocations we have during normal build without errors. Boxing of value types can be omitted by using generic arguments (I don't think custom interpolation handler is supported in NETFx 3.5). It's pretty easy and small change like this:

// There should be overloads also for 0, 1 and 2 arguments
internal static void Trace<T0, T1, T2>(string format, T0 arg0, T1 arg1, T2 arg2)
{
    if (s_trace)
    {
        Trace(string.Format(format, arg0.ToString(), arg1.ToString(), arg2.ToString()));
    }
}

// Replaces Trace(string format, params object[] args)
private static void Trace(string formattedString)
{
    ...
}

[rainersigwald]

I don't think custom interpolation handler is supported in NETFx 3.5

This is true but historically we have prioritized debuggability + modern .NET performance over net35 performance, if it gets to cleaner better code. For instance see my "just copy the whole thing on any write operation" implementation of ImmutableDictionary.

My instinct is that these CommunicationsUtilities methods are called infrequently enough that it doesn't much matter. If we go the custom interpolation handler route I'd rather see it on task logging (#7875).