Remove some allocations from ManifestBuilder.CreateManifestString

[stephentoub]

Just happened to see this low-hanging fruit while looking at some traces. For the RuntimeEventSource, this removes around 30K of allocation, though that's only ~3% of what gets allocated.

Contributes to #44598

cc: @brianrob

[ghost]

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

---

Issue meta data

Issue content:	Just happened to see this low-hanging fruit while looking at some traces. For the RuntimeEventSource, this removes around 30K of allocation, though that's only ~3% of what gets allocated. cc: @brianrob
Issue author:	stephentoub
Assignees:	-
Milestone:	[object Object]

[tarekgh]

CC @noahfalk

[noahfalk]

cc @davmason @sywhang @josalem

[davmason]

Thanks. I'm a little worried because I don't trust our tests to catch any subtle bugs here, but the change is small enough it's probably fine.

[stephentoub]

I'm a little worried because I don't trust our tests to catch any subtle bugs here, but the change is small enough it's probably fine.

Is there something I could do to make us more confident?

[sywhang]

LGTM - as David mentioned we don't have that many tests that catches manifest generation issues (which we really should add) but this change is fairly small and is pretty well understood. Adding @josalem for another pair of eyes to take a look.

[josalem]

This looks good to me. I'd be curious to take a look offline at some of the perf data you collected if it's worth sharing.

[davmason]

I'm a little worried because I don't trust our tests to catch any subtle bugs here, but the change is small enough it's probably fine.

Is there something I could do to make us more confident?

The thing that needs to happen is to add more tests, which is too much to ask of you for this change. I think it's a small enough change that we can merge.

[stephentoub]

Ok, thanks. Just to put my mind at ease (I don't like changing code when we believe tests to not cover it well), I ran:

using System;
using System.Diagnostics.Tracing;

Console.WriteLine(EventSource.GenerateManifest(typeof(object).Assembly.GetType("System.Diagnostics.Tracing.NativeRuntimeEventSource"), null));

before and after this change, and there were no diffs in the output.

[stephentoub]

I'd be curious to take a look offline at some of the perf data you collected if it's worth sharing.

@josalem, here's an example, looking at startup allocations for a dotnet new webapi app: