perf(core): Performance fixes in GameProfilingSystem and ChromeTracingProfileWriter

[froce]

PR Details

I took another look at the profiler changes I made, as @manio143 mentioned a perf issue.
I misunderstood how Utf8JsonWriter works - it won't flush to the underlying stream automatically, so there were constant reallocations. And in GameProfilingSystem checking if it is time to update the output turned into a busy-wait while not subscribed to the Profiler.

Description

• Fix busy-wait in GameProfilingSystem and make surrounding code clearer.
• Add manual flushing to the JsonWriter, so the internal buffer doesn't grow indefinitely and put it on the fast path by skipping validation.

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.

[manio143]

GameProfilingSystem - Would it be possible to separate receiving messages and updating strings into two separate, concurrent loops?
With semaphore slim for synchronization.

I feel like there's a lot of complexity in reading the update method because it behaves one way in Fps mode, and differently in the other modes. containMarks can be made into a field, then we have two loops:

while (!cancellation) {
  await Delay(Refresh)
  await lock
  update strings
  release lock
}

await foreach (var event) {
  await lock
  update state with new event
  release lock
}

I don't think semaphore slim will add too much overhead on top of async here, but of course would be good to verify it doesn't change the perf much.

[froce]

Adding the semaphore adds about 80ns (~400ns -> ~480 ns) per event for me, I guess that would be okay?

Alternatively, how about splitting into one task for fps display and a second task for detailed stats and then changing the active task depending on the active state.

UpdateFpsStats:
while (!cancellation) {
  await Delay(Refresh)
  update strings if needed
}

UpdateDetailedStats:
while (!cancellation) {
  await foreach (var event) {
    update state with new event
    update strings if needed
  }
}

That way the complexity in the update method is also removed from the update method and we don't have to worry about synchronization. But I'm fine with your suggestion too, if you prefer it.

[manio143]

I want it to be readable most of all (you're suggestion is ok).
Maybe having two tasks makes more sense and you can switch between them when switching the display type?

[Kryptos-FR]

Any long-running operation should have a TaskCancellationSource and pass the associated CancellationToken everywhere. Destroy() should not hang on the next RefreshTime tick.

[Kryptos-FR]

Why does it need to wait after Stop() has been called. Can't it just signal a cancellation and return immediately?

[manio143]

I think it would be good to wait here in order to observe any exception which may have been thrown

[Kryptos-FR]

This also need a CancellationTokenSource to stop the task at the earliest opportunity. There should also be a protection against calling Start() or Stop() multiple times on the same instance.

[froce]

Continued in #2050