Properly cache CStrings for plot names

[Mervill]

From the Tracy docs:

3.1 Handling text strings

When dealing with Tracy macros, you will encounter two ways of providing string data to the profiler. In
both cases, you should pass const char* pointers, but there are differences in the expected lifetime of the
pointed data.

    1. When a macro only accepts a pointer (for example: TracyMessageL(text)), the provided string data
must be accessible at any time in program execution (this also includes the time after exiting the main
function). The string also cannot be changed. This basically means that the only option is to use a string
literal (e.g.: TracyMessageL("Hello")).

    2. If there’s a string pointer with a size parameter (for example TracyMessage(text, size)), the profiler
will allocate a temporary internal buffer to store the data. The size count should not include the
terminating null character, using strlen(text) is fine. The pointed-to data is not used afterward.
Remember that allocating and copying memory involved in this operation has a small time cost.

Be aware that every single instance of text string data passed to the profiler can’t be larger than 64 KB.

I had forgotten when I added the Plot wrapper that those functions fall into category 1 above. We need to cache the Plot name CStrings to satisfy Tracy's requirements. That's what this PR does.

Currently the strings are cached and kept privately by the Profiler for the duration of the program. I'm not yet convinced allowing external code to clear the cache is what we want. Let me explain.

The cache clear function would look like this:

    /// <summary>
    /// Clears the Profiler's internal string cache.
    /// </summary>
    public static void ClearCStringCache()
    {
        // done like this in case we want to clear other cache dictionaries in the future.
        DictionaryClearEach(PlotNameCache, x => x.Dispose());
    }

    private static void DictionaryClearEach<TKey, TValue>(Dictionary<TKey, TValue> dict, Action<TValue> each)
    {
        foreach (var key in dict.Keys)
        {
            if (dict.Remove(key, out TValue value))
            {
                each(value);
            }
        }
    }

It was brought up in the other PR that one reason we might want to have support for clearing the cache is because of TRACY_ON_DEMAND which causes Tracy to only store data when the profiler is actually connected. When the profiler is disconnected we might conceivably want to free the cache.

But remember that Tracy wants these strings to be unique for the application lifetime. If you:

1. Connect with the Tracy profiler
2. Receive some profiling data
3. Disconnect Tracy
4. Free the CString cache
5. Reconnect with the Tracy profiler (Requires TRACY_ON_DEMAND)

Then I think Tracy will eat bad data because the pointers are not the same between the two times you've connected with the profiler.

With that in mind, the only time it would make sense to let external code clear the cache when TRACY_ON_DEMAND is active is when we can guarantee that the profiler won't reconnect. Which seems like an extremely rare situation? Other then at program shutdown?

On a similar line of thinking; TRACY_ON_DEMAND prevents sending data inside the native code but you could clear the CString cache on the C# side, call Plot("Foo", 123) again after the profiler is disconnected, and have the Profiler class happily recache the string. Even though it's likely invalid (as discussed above) and even though it may not actually be sent anywhere because the profiler is disconnected. I feel like we would need to add guard statements to the functions that use cached CStrings to prevent this behavior. I could be overthinking it though.

I use this library though the NuGet package so I don't have access to the version of the lib that has TRACY_ON_DEMAND. If you wanted to test that version with the cache clearing code I posted above and test reconnecting, that would be helpful.

Minor changes:

• Exposes the connection status via bool IsConnected()
• Adds .gitignore entries for C# artifacts and .vs files.

Fixes #34

[clibequilibrium]

@Mervill thank you for the PR, would it make sense in this case to make Profiler : IDisposable and in Dispose method have the Dictionary cleared? And on the user side start profiler via using (var profiler = new Profiler()) ?

    public static void Dispose()
    {
        // Free CStrings
        foreach (var kvp in tracyAllocationsDictionary)
        {
            kvp.Value.Item1.Dispose();
            kvp.Value.Item2.Dispose();
        }
    }

[Mervill]

For the reasons I outlined in the OP, I don't think allowing external code to dispose the string cache is appropriate.

If you really want to implement it, I suggest you use the ClearCStringCache() function I provided. You can make a static class implement IDisposable but it's not really semantic to how IDisposable is meant to be used.

In the Dispose() code code you provided the CString wrapper objects and the outer Tuple don't actually get removed from the cache dict. Meaning that it would break if Dispose() was called and the Profiler was re-used.

[clibequilibrium]

For the reasons I outlined in the OP, I don't think allowing external code to dispose the string cache is appropriate.

If you really want to implement it, I suggest you use the ClearCStringCache() function I provided. You can make a static class implement IDisposable but it's not really semantic to how IDisposable is meant to be used.

In the Dispose() code code you provided the CString wrapper objects and the outer Tuple don't actually get removed from the cache dict. Meaning that it would break if Dispose() was called and the Profiler was re-used.

how about making Dispose method private and Profile class not static. The proper usage of Profiler then will be via using statement. Otherwise right now the Profiler leaks memory after it's no longer used?

[Mervill]

The Dispose() method has to be public to correctly implement IDisposable. There's an obscure workaround to this involving explicit interface definitions but that's also not a correct solution.

The Tracy docs are saying that these stings (the plot names) need to be kept around for the entire program lifetime. Even if you use TRACY_ON_DEMAND you need to keep these strings in memory between profiler connections so the profiler data dosen't become invalid.

The only appropriate time to free these strings is at program shutdown, in which case we can simply let the memory be invalidated by the C# runtime as it's shutting down.

[clibequilibrium]

The Dispose() method has to be public to correctly implement IDisposable. There's an obscure workaround to this involving explicit interface definitions but that's also not a correct solution.

The Tracy docs are saying that these stings (the plot names) need to be kept around for the entire program lifetime. Even if you use TRACY_ON_DEMAND you need to keep these strings in memory between profiler connections so the profiler data dosen't become invalid.

The only appropriate time to free these strings is at program shutdown, in which case we can simply let the memory be invalidated by the C# runtime as it's shutting down.

Thanks a lot. Sorry for missing this part